diff --git a/CHANGELOG.md b/CHANGELOG.md index 4e42a1aa..85eaed44 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -26,6 +26,10 @@ plot sizes itself from its container, so a zero-width first measurement drew it at zero size with nothing left to correct it. It now recovers once the container has a real width. +- ggsql interpreter sessions in Positron now come back after an extension host + restart as well as after a window reload. A session the user renamed also + keeps its name across the restore, and ggsql runtimes are rediscovered on + every window open rather than risking a stale cache hit. ## 0.4.1 - 2026-06-22 diff --git a/ggsql-vscode/CLAUDE.md b/ggsql-vscode/CLAUDE.md index 23847792..4f83fe19 100644 --- a/ggsql-vscode/CLAUDE.md +++ b/ggsql-vscode/CLAUDE.md @@ -17,6 +17,7 @@ ggsql-vscode/ ├── src/ │ ├── extension.ts activate(): registers commands, manager, code lenses │ ├── manager.ts Kernel discovery + Positron language-runtime registration +│ ├── positronApi.ts Acquires the Positron API so Positron can attribute it to this extension │ ├── connections.ts Connection-string handling for the Connections pane │ ├── cellParser.ts Splits .ggsql files into cells for Run-Cell commands │ ├── codelens.ts "▶ Run cell" lens above each cell @@ -96,7 +97,13 @@ Outside Positron there is no way to execute a query: `activate()` returns early, - the three keybindings - the five run commands, hidden from the Command Palette via a `commandPalette` menu entry -`isPositron` is declared with a default of `true`, so it is correct in Positron from startup with no code and no activation-order window. In VS Code the key is never declared, so it evaluates falsy. Prefer it over a hand-rolled `setContext` key for anything purely declarative; the Positron API (`tryAcquirePositronApi`) is the right check when the code needs the API object itself. +`isPositron` is declared with a default of `true`, so it is correct in Positron from startup with no code and no activation-order window. In VS Code the key is never declared, so it evaluates falsy. Prefer it over a hand-rolled `setContext` key for anything purely declarative; `getPositronApi()` from [`src/positronApi.ts`](src/positronApi.ts) is the right check when the code needs the API object itself. + +**Acquiring the Positron API.** `src/positronApi.ts` calls `require('positron')` directly, and `esbuild.js` lists `positron` in `external` so the call is still there in `out/extension.js`. This is load bearing, not a style choice. Positron's require interceptor works out which extension owns an API object from the filesystem path of the requiring file, and that identity becomes the `extensionId` on every runtime the extension registers. Reaching the API through the global accessor that `tryAcquirePositronApi()` uses puts the requiring path inside Positron's own bootstrap, which the interceptor cannot place, so the runtime is filed under `nullExtensionDescription` and Positron cannot activate this extension when it restores sessions after a window reload. `src/test/bundle.test.ts` guards the esbuild half of this. + +The Positron Supervisor is a soft dependency, reached through `getSupervisorApi()` in `manager.ts`. It deliberately is not in `extensionDependencies`: that field is static, and an entry for `positron.positron-supervisor` would stop the extension activating at all in VS Code, where the supervisor does not exist. + +`GgsqlRuntimeManager.alwaysRediscover` is `true` because ggsql runtimes are never marked `cacheable`, so Positron must run discovery on every window open rather than trusting its cross-window cache. The typings declare it as an optional member, so `tsc` checks the value's type but not the name: a misspelling would compile as a harmless extra property and silently disable the flag. `src/test/manager.test.ts` is the guard, because the property access there fails to compile if the name changes. Anything that does *not* need the runtime (`ggsql.createNewFile`, `ggsql.resetSqlAssociationPrompt`, syntax highlighting) is registered before the early return and works in plain VS Code. Add new commands on the correct side of that line, and gate them if they execute code. @@ -121,6 +128,8 @@ code --install-extension ggsql-.vsix Watch mode for development: `npm run watch` (runs esbuild + tsc in parallel). +For an interactive session, open the **repo root** in Positron and press F5 ("Run Extension"). [`/.vscode/launch.json`](../.vscode/launch.json) runs the `build-ggsql-vscode` task, which is `npm run watch` in this folder, then opens an Extension Development Host with `--extensionDevelopmentPath`, so the extension loads from source with no VSIX. Launch from Positron rather than VS Code, or the dev host has no Positron API and the runtime manager never registers. The watcher rebuilds `out/extension.js` on save, but the host does not hot-reload: run _Developer: Reload Window_ in the Extension Development Host to pick up a change. + ## Testing ```sh @@ -134,7 +143,7 @@ Tests live in `src/test/` and compile to `out-test/` via `tsconfig.test.json`, d Note that `tsc` does not prune output for deleted sources: if you delete or rename a test, remove its `.js` and `.js.map` from `out-test/test/` or the runner keeps executing the stale copy. `npm run test:extension` on its own does not recompile, so run `npm test` (or `npm run compile-tests` first) after editing any `.ts`. -The suites cover the extension as stock VS Code sees it: activation, language resolution, cell parsing, `.sql` gating, CodeLens placement and TextMate scopes. The Positron surface (runtime manager, connection drivers, cell execution) is not covered, since it needs a Positron host. `sqlAssociation.ts`, `manager.ts` and `connections.ts` are also untested. +The suites cover the extension as stock VS Code sees it: activation, language resolution, cell parsing, `.sql` gating, CodeLens placement, TextMate scopes, and the parts of `manager.ts` and `positronApi.ts` that are reachable without a Positron host. `bundle.test.ts` additionally asserts against the built `out/extension.js`. The rest of the Positron surface (session creation, connection drivers, cell execution) is not covered, since it needs a Positron host, and `sqlAssociation.ts` and `connections.ts` are untested. Add new tests as `src/test/.test.ts`; no config change is needed. diff --git a/ggsql-vscode/esbuild.js b/ggsql-vscode/esbuild.js index c49e0642..4e7f052b 100644 --- a/ggsql-vscode/esbuild.js +++ b/ggsql-vscode/esbuild.js @@ -13,7 +13,7 @@ async function main() { sourcesContent: false, platform: 'node', outfile: 'out/extension.js', - external: ['vscode'], + external: ['vscode', 'positron'], logLevel: 'info', }); diff --git a/ggsql-vscode/package-lock.json b/ggsql-vscode/package-lock.json index dc5829a6..8e8db899 100644 --- a/ggsql-vscode/package-lock.json +++ b/ggsql-vscode/package-lock.json @@ -12,7 +12,7 @@ "toml": "^3.0.0" }, "devDependencies": { - "@posit-dev/positron": "^0.2.2", + "@posit-dev/positron": "^0.2.7", "@types/mocha": "^10.0.10", "@types/node": "^18.x", "@types/vscode": "^1.75.0", @@ -753,9 +753,9 @@ } }, "node_modules/@posit-dev/positron": { - "version": "0.2.2", - "resolved": "https://registry.npmjs.org/@posit-dev/positron/-/positron-0.2.2.tgz", - "integrity": "sha512-MjNHoZJKUHafwVSI5fAb7i4mLWEjptHxo0fNGfLbeAKT4RAoifVIiF5yCPz/rKIBH8xYQrAb383vcDA2Kzy2ZQ==", + "version": "0.2.7", + "resolved": "https://registry.npmjs.org/@posit-dev/positron/-/positron-0.2.7.tgz", + "integrity": "sha512-BNTWBi3IshSbtbVS7WdgbhGeHCv2NNpiWjB+ovll6GcdLCOtu5MTeAxPwddCZOg7xSSKwq8s+l4f+5ANPExBFw==", "dev": true, "license": "MIT", "engines": { diff --git a/ggsql-vscode/package.json b/ggsql-vscode/package.json index 3282491f..f11f0b5f 100644 --- a/ggsql-vscode/package.json +++ b/ggsql-vscode/package.json @@ -191,7 +191,7 @@ "toml": "^3.0.0" }, "devDependencies": { - "@posit-dev/positron": "^0.2.2", + "@posit-dev/positron": "^0.2.7", "@types/mocha": "^10.0.10", "@types/node": "^18.x", "@types/vscode": "^1.75.0", diff --git a/ggsql-vscode/src/extension.ts b/ggsql-vscode/src/extension.ts index beefcacf..435f26a0 100644 --- a/ggsql-vscode/src/extension.ts +++ b/ggsql-vscode/src/extension.ts @@ -6,7 +6,7 @@ */ import * as vscode from 'vscode'; -import { tryAcquirePositronApi } from '@posit-dev/positron'; +import { getPositronApi } from './positronApi'; import { GgsqlRuntimeManager } from './manager'; import { createConnectionDrivers } from './connections'; import { GgsqlCodeLensProvider, registerCellCommands } from './codelens'; @@ -43,7 +43,7 @@ export function activate(context: vscode.ExtensionContext): void { activateSqlAssociationPrompt(context); // Try to acquire the Positron API - const positronApi = tryAcquirePositronApi(); + const positronApi = getPositronApi(); if (!positronApi) { // Running in VS Code (not Positron) - syntax highlighting still works diff --git a/ggsql-vscode/src/manager.ts b/ggsql-vscode/src/manager.ts index 9aa84bf7..5c781118 100644 --- a/ggsql-vscode/src/manager.ts +++ b/ggsql-vscode/src/manager.ts @@ -307,23 +307,57 @@ function ensureKernelSpecInstalled(kernelPath: string): void { /** * Create the dynamic state for a ggsql runtime session. + * + * @param sessionName The name Positron holds for the session, when restoring + * one. New sessions have no name yet and get the default. */ -function createDynState(): positron.LanguageRuntimeDynState { +export function createDynState(sessionName?: string): positron.LanguageRuntimeDynState { return { inputPrompt: 'ggsql> ', continuationPrompt: '... ', - sessionName: 'ggsql', + sessionName: sessionName || 'ggsql', }; } +/** + * Get the Positron Supervisor API, activating the extension if needed. + * + * The supervisor is a soft dependency: it is declared nowhere in + * package.json, because an extensionDependencies entry would stop this + * extension activating at all in VS Code, where the supervisor does not + * exist. Awaiting activate() here gives the same ordering guarantee that a + * declared dependency would. + */ +export async function getSupervisorApi(): Promise { + const supervisorExt = vscode.extensions.getExtension( + 'positron.positron-supervisor' + ); + + if (!supervisorExt) { + throw new Error('Positron Supervisor extension not found'); + } + + return supervisorExt.activate(); +} + /** * ggsql Language Runtime Manager * * Manages the lifecycle of ggsql runtime sessions in Positron. */ export class GgsqlRuntimeManager implements positron.LanguageRuntimeManager { + /** + * Run discovery on every window open rather than trusting Positron's + * cross-window cache. + * + * ggsql runtimes are not marked cacheable: the ggsql.kernelPath setting is + * workspace scoped, and the PATH fallback is not guaranteed to resolve to + * a real file. A cache hit would therefore register only some of the + * candidates and silently hide the rest on warm starts. + */ + public readonly alwaysRediscover = true; + private _context: vscode.ExtensionContext; - private _sessions: Map = new Map(); constructor(context: vscode.ExtensionContext) { this._context = context; @@ -383,17 +417,7 @@ export class GgsqlRuntimeManager implements positron.LanguageRuntimeManager { runtimeMetadata: positron.LanguageRuntimeMetadata, sessionMetadata: positron.RuntimeSessionMetadata ): Promise { - // Get the Positron Supervisor extension - const supervisorExt = vscode.extensions.getExtension( - 'positron.positron-supervisor' - ); - - if (!supervisorExt) { - throw new Error('Positron Supervisor extension not found'); - } - - // Ensure the extension is activated - const supervisorApi = await supervisorExt.activate(); + const supervisorApi = await getSupervisorApi(); // Create the kernel spec using the runtime's kernel path const kernelSpec = createKernelSpec(runtimeMetadata.runtimePath); @@ -411,14 +435,6 @@ export class GgsqlRuntimeManager implements positron.LanguageRuntimeManager { dynState ); - // Track the session - this._sessions.set(sessionMetadata.sessionId, session); - - // Remove from tracking when session ends - session.onDidEndSession(() => { - this._sessions.delete(sessionMetadata.sessionId); - }); - return session; } @@ -427,20 +443,12 @@ export class GgsqlRuntimeManager implements positron.LanguageRuntimeManager { */ async restoreSession( runtimeMetadata: positron.LanguageRuntimeMetadata, - sessionMetadata: positron.RuntimeSessionMetadata + sessionMetadata: positron.RuntimeSessionMetadata, + sessionName: string ): Promise { - // Get the Positron Supervisor extension - const supervisorExt = vscode.extensions.getExtension( - 'positron.positron-supervisor' - ); - - if (!supervisorExt) { - throw new Error('Positron Supervisor extension not found'); - } - - const supervisorApi = await supervisorExt.activate(); + const supervisorApi = await getSupervisorApi(); - const dynState = createDynState(); + const dynState = createDynState(sessionName); // Re-advertise this kernel on restore ensureKernelSpecInstalled(runtimeMetadata.runtimePath); @@ -451,12 +459,6 @@ export class GgsqlRuntimeManager implements positron.LanguageRuntimeManager { dynState ); - this._sessions.set(sessionMetadata.sessionId, session); - - session.onDidEndSession(() => { - this._sessions.delete(sessionMetadata.sessionId); - }); - return session; } @@ -464,15 +466,7 @@ export class GgsqlRuntimeManager implements positron.LanguageRuntimeManager { * Validate an existing session. */ async validateSession(sessionId: string): Promise { - const supervisorExt = vscode.extensions.getExtension( - 'positron.positron-supervisor' - ); - - if (!supervisorExt) { - return false; - } - - const supervisorApi = await supervisorExt.activate(); + const supervisorApi = await getSupervisorApi(); return supervisorApi.validateSession(sessionId); } } diff --git a/ggsql-vscode/src/positronApi.ts b/ggsql-vscode/src/positronApi.ts new file mode 100644 index 00000000..5cb9ec36 --- /dev/null +++ b/ggsql-vscode/src/positronApi.ts @@ -0,0 +1,38 @@ +/* + * Positron API access. + * + * Positron's require interceptor decides which extension owns an API object + * by matching the filesystem path of the file that called require('positron') + * against its map of extension folders. That identity becomes the extensionId + * on every runtime this extension registers, and Positron uses it to activate + * the owning extension when it restores sessions after a window reload. + * + * The require therefore has to happen in a ggsql source file, and 'positron' + * is marked external in esbuild.js so the call is still in out/extension.js + * rather than inlined. Reaching the API through a global accessor instead + * attributes it to Positron's own bootstrap file, which the interceptor + * cannot place, and the runtime is recorded under nullExtensionDescription. + * + * In VS Code the module does not exist, so the require throws and the + * extension runs without the Positron surface. + */ + +import type { PositronApi } from '@posit-dev/positron'; + +let api: PositronApi | undefined; +let attempted = false; + +/** + * Get the Positron API, or undefined when not running in Positron. + */ +export function getPositronApi(): PositronApi | undefined { + if (!attempted) { + attempted = true; + try { + api = require('positron') as PositronApi; + } catch { + // Not running in Positron. + } + } + return api; +} diff --git a/ggsql-vscode/src/test/bundle.test.ts b/ggsql-vscode/src/test/bundle.test.ts new file mode 100644 index 00000000..fe76f9db --- /dev/null +++ b/ggsql-vscode/src/test/bundle.test.ts @@ -0,0 +1,25 @@ +import * as assert from 'assert'; +import * as fs from 'fs'; +import * as path from 'path'; + +// out-test/test/bundle.test.js -> the extension root +const bundlePath = path.resolve(__dirname, '..', '..', 'out', 'extension.js'); + +suite('bundle', () => { + test('keeps positron as an external require', () => { + // Positron's require interceptor attributes the API object by the path + // of the requiring file. The call has to survive bundling and stay in + // out/extension.js, which lives inside the extension folder. If esbuild + // inlines the module instead, every registered runtime is recorded + // under nullExtensionDescription and session restore breaks. + assert.ok(fs.existsSync(bundlePath), 'out/extension.js missing; run npm run package first'); + const bundle = fs.readFileSync(bundlePath, 'utf8'); + assert.match(bundle, /require\(["']positron["']\)/); + }); + + test('does not bundle the positron API helper package', () => { + assert.ok(fs.existsSync(bundlePath), 'out/extension.js missing; run npm run package first'); + const bundle = fs.readFileSync(bundlePath, 'utf8'); + assert.doesNotMatch(bundle, /acquirePositronApi/); + }); +}); diff --git a/ggsql-vscode/src/test/manager.test.ts b/ggsql-vscode/src/test/manager.test.ts new file mode 100644 index 00000000..812b0daa --- /dev/null +++ b/ggsql-vscode/src/test/manager.test.ts @@ -0,0 +1,53 @@ +import * as assert from 'assert'; +import * as vscode from 'vscode'; +import { GgsqlRuntimeManager, createDynState, getSupervisorApi } from '../manager'; + +suite('manager', () => { + // The suites run in stock VS Code, where positron.positron-supervisor is + // not installed. All three call sites depend on this rejecting rather than + // resolving with a partially usable object. + test('getSupervisorApi rejects when the supervisor is absent', async () => { + await assert.rejects( + () => getSupervisorApi(), + /Positron Supervisor extension not found/, + ); + }); + + test('createDynState falls back to the default session name', () => { + const state = createDynState(); + assert.strictEqual(state.sessionName, 'ggsql'); + assert.strictEqual(state.inputPrompt, 'ggsql> '); + assert.strictEqual(state.continuationPrompt, '... '); + }); + + test('createDynState keeps a name Positron supplies', () => { + // Positron passes the current session name to restoreSession. Dropping + // it renames the console back to 'ggsql' on every window reload. + const state = createDynState('Sales analysis'); + assert.strictEqual(state.sessionName, 'Sales analysis'); + }); + + test('createDynState falls back when the supplied name is empty', () => { + // A blank name would otherwise leave the restored console with no title. + assert.strictEqual(createDynState('').sessionName, 'ggsql'); + }); + + test('the manager opts out of the discovery cache fast path', () => { + // ggsql runtimes are never marked cacheable, because the kernel path + // can come from a workspace setting or from PATH. Without this flag + // Positron would be free to skip discovery on a warm start and leave + // ggsql unregistered. + const manager = new GgsqlRuntimeManager({} as vscode.ExtensionContext); + assert.strictEqual(manager.alwaysRediscover, true); + }); + + test('restoreSession propagates a missing supervisor', async () => { + // getSupervisorApi() is awaited first, before any kernel spec is + // written, so the rejection arrives with nothing done on disk. + const manager = new GgsqlRuntimeManager({} as vscode.ExtensionContext); + await assert.rejects( + () => manager.restoreSession({} as never, {} as never, 'Sales analysis'), + /Positron Supervisor extension not found/, + ); + }); +}); diff --git a/ggsql-vscode/src/test/positronApi.test.ts b/ggsql-vscode/src/test/positronApi.test.ts new file mode 100644 index 00000000..5a34f6f1 --- /dev/null +++ b/ggsql-vscode/src/test/positronApi.test.ts @@ -0,0 +1,10 @@ +import * as assert from 'assert'; +import { getPositronApi } from '../positronApi'; + +suite('positronApi', () => { + // The suites run in stock VS Code, where the 'positron' module does not + // exist. Activation calls this on every host, so it must not throw here. + test('returns undefined outside Positron instead of throwing', () => { + assert.strictEqual(getPositronApi(), undefined); + }); +});