From 199b5b1d4e94b6af9de687acd69e5d85546cc393 Mon Sep 17 00:00:00 2001 From: Sam Clark Date: Tue, 18 Aug 2026 10:07:38 -0500 Subject: [PATCH 1/8] Discover a kernel bundled inside the extension The per-platform VSIXes will carry ggsql-jupyter at bundled/bin/, so installing the extension is enough. Add a ggsql.kernelStrategy setting (bundled | environment | path, modelled on air.executableStrategy) that decides where manager.ts looks; a ggsql.kernelPath configured before the strategy existed still means "use that path". Also fix the phantom runtime: discovery appended the bare binary name whether or not it was on PATH, and the accessibility check accepted any non-absolute path, so a machine with no kernel got a registered runtime that failed at session start with KS-19. With no kernel and no bundle, discovery now yields nothing. The bundled kernel gets a fixed runtimeId rather than one hashed from its path, which contains the versioned extension directory: hashing it would mint a new runtime on every update and lose runtime affinity and restorable sessions. Co-Authored-By: Claude Opus 5 (1M context) --- ggsql-vscode/.gitignore | 1 + ggsql-vscode/CLAUDE.md | 35 ++- ggsql-vscode/package.json | 17 +- ggsql-vscode/src/manager.ts | 287 +++++++++++++---- ggsql-vscode/src/test/kernelDiscovery.test.ts | 294 ++++++++++++++++++ 5 files changed, 566 insertions(+), 68 deletions(-) create mode 100644 ggsql-vscode/src/test/kernelDiscovery.test.ts diff --git a/ggsql-vscode/.gitignore b/ggsql-vscode/.gitignore index 7eaa3d35..6f6ad1ae 100644 --- a/ggsql-vscode/.gitignore +++ b/ggsql-vscode/.gitignore @@ -1,3 +1,4 @@ out out-test .vscode-test/ +bundled diff --git a/ggsql-vscode/CLAUDE.md b/ggsql-vscode/CLAUDE.md index 4f83fe19..db3cf0e0 100644 --- a/ggsql-vscode/CLAUDE.md +++ b/ggsql-vscode/CLAUDE.md @@ -28,6 +28,7 @@ ggsql-vscode/ │ └── test/ Mocha suites (unit + activation) and the grammar fixture ├── syntaxes/ │ └── ggsql.tmLanguage.json TextMate grammar (used for tokenization in VS Code) +├── bundled/bin/ Kernel shipped inside the platform VSIXes (staged at release time, not in git) ├── examples/ Sample .ggsql files ├── resources/ Static assets bundled with the extension │ ├── ggsql-icon.svg Full-colour logo; read by manager.ts for base64EncodedIconSvg @@ -87,8 +88,8 @@ The `ggsql.enableSqlFiles` description uses `markdownDescription` rather than `d The extension declares `contributes.languageRuntimes` for `ggsql` (see `package.json`) and depends on `@posit-dev/positron`. When activated under Positron, `manager.ts`: -1. Discovers a `ggsql-jupyter` binary via, in order: the `ggsql.kernelPath` setting, an installed Jupyter kernelspec named `ggsql`, or `ggsql-jupyter` on `PATH`. -2. Registers it as a Positron language runtime so `▶ Run` and the Console route to the kernel. +1. Discovers `ggsql-jupyter` binaries as described in [Finding the kernel](#finding-the-kernel) below. +2. Registers each as a Positron language runtime so `▶ Run` and the Console route to the kernel. 3. Routes plot output to Positron's Plot pane via metadata coming back from the kernel (`output_location: "plot"`). Outside Positron there is no way to execute a query: `activate()` returns early, so every command that runs code stays unregistered. To avoid offering actions that cannot work, everything execution-related gates on Positron's built-in **`isPositron`** context key ([extension development docs](https://positron.posit.co/extension-development.html#option-1-context-keys)): @@ -107,11 +108,37 @@ The Positron Supervisor is a soft dependency, reached through `getSupervisorApi( 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. +## Finding the kernel + +The extension ships the kernel: the per-platform VSIXes carry `ggsql-jupyter` at `bundled/bin/`, so installing the extension is enough and no native installer is needed. The platform-neutral VSIX carries none, and users on a platform without a build install the kernel themselves. + +`ggsql.kernelStrategy` decides where `manager.ts` looks, modelled on `air.executableStrategy`: + +| Strategy | Candidates, in priority order | +| --- | --- | +| `bundled` (default) | The bundled kernel alone. A build that carries none falls through to the host locations, so the platform-neutral VSIX behaves as it always did. | +| `environment` | Host locations, then the bundled kernel as the fallback. | +| `path` | `ggsql.kernelPath` alone — neither the bundled kernel nor a host install stands in for it. An empty path is treated as `bundled`. | + +Host locations are, in order: Jupyter kernelspec directories (user then system), the native package install locations per platform, then `PATH`. + +`selectKernelCandidates()` is the whole precedence rule with no filesystem in it, which is what `src/test/kernelDiscovery.test.ts` exercises; `discoverKernelPaths()` supplies it with what is actually on disk. + +Four things here are load bearing: + +- **Every candidate is an absolute path.** A candidate that is only a binary name satisfies each existence check further down and so registers a runtime that fails at session start with `KS-19: Kernel path not found`. `findOnPath()` returns `undefined` rather than the bare name, and `isKernelAccessible()` rejects any non-absolute path, so no kernel anywhere means **zero** runtimes rather than an unusable one. The single exception is a `ggsql.kernelPath` that resolves to nothing: it is passed through so discovery can report it as inaccessible in the log instead of ignoring the setting silently. +- **The bundled kernel's `runtimeId` is fixed, not derived from its path.** Every other source hashes `kernelPath` to get one id per installed kernel, but the bundled path contains the versioned extension directory, so hashing it would mint a new runtime on every extension update and lose the workspace's runtime affinity and its restorable sessions. +- **The bundled runtime is named plain `ggsql`.** The `ggsql ()` suffix is only worth showing for a kernel the user went out of their way to select. +- **`ggsql.kernelPath` implies `path`.** Users configured that setting before a strategy existed, so a non-empty path with no explicitly set `kernelStrategy` still resolves to `path`. `resolveKernelStrategy()` reads the value through `inspect()` for that reason: `get()` cannot tell a set value from the default. + +Discovery also writes the user-level Jupyter kernelspec for the bundled and system kernels, so Quarto and Jupyter can find ggsql without a session ever being started, and so the spec stops pointing into an extension directory an update has removed. + ## Settings ```json { - "ggsql.kernelPath": "string" // empty → use 'ggsql-jupyter' from PATH + "ggsql.kernelStrategy": "bundled" | "environment" | "path", // default "bundled" + "ggsql.kernelPath": "string" // used when the strategy is "path" } ``` @@ -143,7 +170,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, 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. +The suites cover the extension as stock VS Code sees it: activation, language resolution, cell parsing, `.sql` gating, CodeLens placement, TextMate scopes, kernel discovery, 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/package.json b/ggsql-vscode/package.json index f11f0b5f..0df030d9 100644 --- a/ggsql-vscode/package.json +++ b/ggsql-vscode/package.json @@ -159,10 +159,25 @@ "type": "object", "title": "ggsql", "properties": { + "ggsql.kernelStrategy": { + "type": "string", + "default": "bundled", + "enum": [ + "bundled", + "environment", + "path" + ], + "enumDescriptions": [ + "Always use the ggsql-jupyter kernel bundled with this extension.", + "Use a ggsql-jupyter kernel installed on this machine, falling back to the bundled kernel.", + "Use the ggsql-jupyter kernel at the path given by ggsql.kernelPath." + ], + "markdownDescription": "How to locate the `ggsql-jupyter` kernel that runs your queries. The bundled kernel ships inside the extension, so no separate install is needed. Setting `#ggsql.kernelPath#` without setting this implies `path`." + }, "ggsql.kernelPath": { "type": "string", "default": "", - "description": "Path to the ggsql-jupyter executable. If empty, uses 'ggsql-jupyter' from PATH." + "markdownDescription": "Path to the `ggsql-jupyter` executable, used when `#ggsql.kernelStrategy#` is `path`. A bare name is looked up on `PATH`." }, "ggsql.enableSqlFiles": { "type": "boolean", diff --git a/ggsql-vscode/src/manager.ts b/ggsql-vscode/src/manager.ts index 5c781118..6f0f5262 100644 --- a/ggsql-vscode/src/manager.ts +++ b/ggsql-vscode/src/manager.ts @@ -15,65 +15,121 @@ import type { JupyterKernelSpec, PositronSupervisorApi } from './types'; import { log } from './extension'; /** Where a kernel candidate was discovered */ -type KernelSource = 'Setting' | 'Jupyter' | 'System' | 'Path'; +type KernelSource = 'Bundled' | 'Setting' | 'Jupyter' | 'System' | 'Path'; + +/** + * How to pick a kernel, from the `ggsql.kernelStrategy` setting. + * + * - `bundled`: the kernel shipped inside the extension. + * - `environment`: a kernel installed on the machine, falling back to the + * bundled one. + * - `path`: the binary named by `ggsql.kernelPath`. + */ +export type KernelStrategy = 'bundled' | 'environment' | 'path'; + +const KERNEL_STRATEGIES: readonly string[] = ['bundled', 'environment', 'path']; /** * A discovered ggsql-jupyter kernel candidate */ -interface KernelCandidate { - /** Absolute path to the ggsql-jupyter binary (or bare name for PATH fallback) */ +export interface KernelCandidate { + /** Path to the ggsql-jupyter binary */ kernelPath: string; /** Human-readable label for where this was found */ source: KernelSource; } +/** Platform-specific file name of the kernel executable */ +function kernelBinaryName(): string { + return process.platform === 'win32' ? 'ggsql-jupyter.exe' : 'ggsql-jupyter'; +} + /** - * Try to resolve a binary name to its absolute path via the system PATH. - * Returns the original value if resolution fails or the path is already absolute. + * Look a binary up on the system PATH. + * + * Returns undefined when it is not there. Callers must not fall back to the + * bare name: a bare name satisfies every existence check further down and so + * registers a runtime that cannot start. */ -function resolveToAbsolutePath(binaryPath: string): string { - if (path.isAbsolute(binaryPath)) { - return binaryPath; - } +function findOnPath(binaryName: string): string | undefined { try { const cmd = process.platform === 'win32' ? 'where' : 'which'; - const resolved = cp.execFileSync(cmd, [binaryPath], { + const resolved = cp.execFileSync(cmd, [binaryName], { encoding: 'utf8', timeout: 5000, }).trim().split(/\r?\n/)[0]; if (resolved && path.isAbsolute(resolved)) { - log(`Resolved '${binaryPath}' to '${resolved}'`); + log(`Resolved '${binaryName}' to '${resolved}'`); return resolved; } } catch { - log(`Could not resolve '${binaryPath}' to absolute path, using as-is`); + // which/where exit non-zero when the name is not on PATH } - return binaryPath; + log(`'${binaryName}' is not on PATH`); + return undefined; } /** - * Discover all available ggsql-jupyter kernel binaries + * Absolutise `ggsql.kernelPath`. * - * Checks in priority order: - * 1. Configured path in settings - * 2. Jupyter kernelspec locations (user and system) - * 3. Cargo-packager install locations - * 4. Fall back to PATH + * A bare name is looked up on PATH; if that fails the configured value is kept + * as-is, so that discovery rejects it as inaccessible and logs it back to the + * user rather than silently ignoring the setting. + */ +function resolveConfiguredPath(configuredPath: string): string { + if (path.isAbsolute(configuredPath)) { + return configuredPath; + } + return findOnPath(configuredPath) ?? configuredPath; +} + +/** + * Restore the executable bit on the bundled kernel if it is missing. * - * Returns deduplicated candidates, keeping the highest-priority occurrence. + * `vsce` preserves the bit through package and install, so this should never + * fire; it is insurance against an unpack that drops it, which would otherwise + * present as the bundled kernel silently not being discovered. */ -function discoverKernelPaths(): KernelCandidate[] { - const candidates: KernelCandidate[] = []; - const binaryName = process.platform === 'win32' ? 'ggsql-jupyter.exe' : 'ggsql-jupyter'; +function ensureExecutable(binaryPath: string): void { + if (process.platform === 'win32') { + return; + } + try { + fs.accessSync(binaryPath, fs.constants.X_OK); + return; + } catch { + // Fall through and try to fix it + } + try { + fs.chmodSync(binaryPath, fs.statSync(binaryPath).mode | 0o111); + log(`Restored the executable bit on ${binaryPath}`); + } catch (err) { + log(`Could not make ${binaryPath} executable: ${err}`); + } +} - // 1. User-configured setting (highest priority) - const config = vscode.workspace.getConfiguration('ggsql'); - const configuredPath = config.get('kernelPath', ''); - if (configuredPath && configuredPath.trim() !== '') { - candidates.push({ kernelPath: configuredPath, source: 'Setting' }); +/** + * Path to the kernel shipped inside the extension, or undefined for a build + * that carries none (the platform-neutral VSIX). + */ +function bundledKernelPath(context: vscode.ExtensionContext): string | undefined { + const bundled = path.join(context.extensionPath, 'bundled', 'bin', kernelBinaryName()); + if (!fs.existsSync(bundled)) { + return undefined; } + ensureExecutable(bundled); + return bundled; +} - // 2. Jupyter kernelspec locations +/** + * Find kernels installed on the machine: Jupyter kernelspec locations, then the + * install locations of the native packages, then PATH. + */ +function discoverHostKernels(): KernelCandidate[] { + const candidates: KernelCandidate[] = []; + const binaryName = kernelBinaryName(); + + // Jupyter kernelspec locations const homeDir = process.env.HOME || process.env.USERPROFILE || ''; const kernelspecPaths = [ // User kernelspec (macOS) @@ -96,7 +152,7 @@ function discoverKernelPaths(): KernelCandidate[] { } } - // 3. Cargo-packager install locations + // Cargo-packager install locations const packagerPaths: string[] = []; if (process.platform === 'darwin') { // PKG installer (current) @@ -120,18 +176,80 @@ function discoverKernelPaths(): KernelCandidate[] { } } - // 4. PATH fallback (last resort) - candidates.push({ kernelPath: resolveToAbsolutePath(binaryName), source: 'Path' }); + // PATH, last of the host locations + const onPath = findOnPath(binaryName); + if (onPath) { + candidates.push({ kernelPath: onPath, source: 'Path' }); + } + + return candidates; +} + +/** + * Resolve `ggsql.kernelStrategy`. + * + * Migration for users who configured `ggsql.kernelPath` before the strategy + * setting existed: a non-empty path with no explicitly set strategy still + * means "use that path", so their setting keeps working untouched. + */ +export function resolveKernelStrategy(config: vscode.WorkspaceConfiguration): KernelStrategy { + const inspected = config.inspect('kernelStrategy'); + const explicit = inspected?.workspaceFolderValue + ?? inspected?.workspaceValue + ?? inspected?.globalValue; + + if (explicit !== undefined) { + if (KERNEL_STRATEGIES.includes(explicit)) { + return explicit as KernelStrategy; + } + log(`Ignoring unknown ggsql.kernelStrategy '${explicit}'`); + } else if (config.get('kernelPath', '').trim() !== '') { + return 'path'; + } + + return 'bundled'; +} - // Deduplicate by resolved absolute path +/** + * Apply a strategy to the places a kernel can come from. + * + * `hostKernels` is a callback so that the common case — the bundled kernel with + * the default strategy — does not pay for a PATH lookup it will not use. + */ +export function selectKernelCandidates( + strategy: KernelStrategy, + bundledPath: string | undefined, + configuredPath: string | undefined, + hostKernels: () => KernelCandidate[], +): KernelCandidate[] { + const bundled: KernelCandidate[] = bundledPath + ? [{ kernelPath: bundledPath, source: 'Bundled' }] + : []; + + if (strategy === 'path') { + if (configuredPath) { + return [{ kernelPath: configuredPath, source: 'Setting' }]; + } + // Nothing to point at. Treat it as the default rather than registering + // no runtime at all. + log('ggsql.kernelStrategy is "path" but ggsql.kernelPath is empty; using the bundled kernel'); + } else if (strategy === 'environment') { + return [...hostKernels(), ...bundled]; + } + + // A build that carries no kernel still looks for a host install, or it + // would offer nothing at all. + return bundled.length > 0 ? bundled : hostKernels(); +} + +/** + * Drop candidates that name a file an earlier candidate already named, keeping + * the highest-priority occurrence. + */ +function dedupeCandidates(candidates: KernelCandidate[]): KernelCandidate[] { const seen = new Set(); const deduped: KernelCandidate[] = []; for (const candidate of candidates) { - if (!path.isAbsolute(candidate.kernelPath)) { - // Non-absolute paths (PATH fallback) can't be deduplicated - deduped.push(candidate); - continue; - } let resolved: string; try { resolved = fs.realpathSync(candidate.kernelPath); @@ -145,32 +263,72 @@ function discoverKernelPaths(): KernelCandidate[] { log(`Skipping duplicate kernel path: ${candidate.kernelPath} (resolves to ${resolved})`); } } - return deduped; } /** - * Check if a kernel executable exists and is accessible + * Discover the ggsql-jupyter kernels this window should offer, in priority + * order. */ -async function isKernelAccessible(kernelPath: string): Promise { - if (path.isAbsolute(kernelPath)) { - try { - await fs.promises.access(kernelPath, fs.constants.X_OK); - return true; - } catch { +export function discoverKernelPaths(context: vscode.ExtensionContext): KernelCandidate[] { + const config = vscode.workspace.getConfiguration('ggsql'); + const strategy = resolveKernelStrategy(config); + log(`Kernel strategy: ${strategy}`); + + const configuredPath = config.get('kernelPath', '').trim(); + + return dedupeCandidates(selectKernelCandidates( + strategy, + bundledKernelPath(context), + configuredPath === '' ? undefined : resolveConfiguredPath(configuredPath), + discoverHostKernels, + )); +} + +/** + * Check that a candidate is a file this process can execute. + * + * A path that is not absolute is rejected: discovery absolutises every source + * it can, so a bare name reaching here means the PATH lookup failed, and + * accepting it would register a runtime that fails at session start. + */ +export async function isKernelAccessible(kernelPath: string): Promise { + if (!path.isAbsolute(kernelPath)) { + return false; + } + try { + const stats = await fs.promises.stat(kernelPath); + if (!stats.isFile()) { return false; } + await fs.promises.access(kernelPath, fs.constants.X_OK); + return true; + } catch { + return false; } +} - // For non-absolute paths (relying on PATH), always return true - // and let the actual kernel startup fail with a proper error message - return true; +/** + * Stable runtime identifier for a candidate. + * + * Hashing the path gives one identifier per installed kernel, which is what + * Positron needs to keep runtime affinity and restorable sessions across + * windows. The bundled kernel lives inside the versioned extension directory, + * so its path changes on every extension update: it gets a fixed identifier + * instead, or each update would look like a different runtime. + */ +function runtimeIdFor(candidate: KernelCandidate): string { + if (candidate.source === 'Bundled') { + return 'ggsql-bundled'; + } + const pathHash = crypto.createHash('sha256').update(candidate.kernelPath).digest('hex').substring(0, 12); + return `ggsql-${pathHash}`; } /** * Generate runtime metadata for a ggsql kernel candidate */ -function generateMetadata( +export function generateMetadata( context: vscode.ExtensionContext, candidate: KernelCandidate, ): positron.LanguageRuntimeMetadata { @@ -179,11 +337,12 @@ function generateMetadata( const iconPath = path.join(context.extensionPath, 'resources', 'ggsql-icon.svg'); const base64Icon = fs.readFileSync(iconPath).toString('base64'); - const pathHash = crypto.createHash('sha256').update(candidate.kernelPath).digest('hex').substring(0, 12); return { - runtimeId: `ggsql-${pathHash}`, + runtimeId: runtimeIdFor(candidate), runtimePath: candidate.kernelPath, - runtimeName: `ggsql (${candidate.source})`, + // The bundled kernel is the default, so it is just "ggsql". Only a + // kernel the user went out of their way to use is worth qualifying. + runtimeName: candidate.source === 'Bundled' ? 'ggsql' : `ggsql (${candidate.source})`, runtimeShortName: 'ggsql', runtimeVersion: version, runtimeSource: 'ggsql', @@ -350,10 +509,10 @@ 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. + * ggsql runtimes are not marked cacheable: ggsql.kernelStrategy and + * ggsql.kernelPath are workspace scoped, and the host kernels a machine + * offers change as packages come and go. A cache hit would therefore + * register a stale set of candidates on warm starts. */ public readonly alwaysRediscover = true; @@ -374,16 +533,18 @@ export class GgsqlRuntimeManager implements positron.LanguageRuntimeManager { const generator = async function* discoverGgsqlRuntimes() { log('Discovering ggsql runtimes...'); - const candidates = discoverKernelPaths(); + const candidates = discoverKernelPaths(context); log(`Found ${candidates.length} kernel candidate(s)`); for (const candidate of candidates) { const accessible = await isKernelAccessible(candidate.kernelPath); if (accessible) { - // When a system install is found, write the kernel spec to - // the user kernelspec dir immediately so that Quarto/Jupyter - // can discover ggsql even if no session is ever started. - if (candidate.source === 'System') { + // Write the kernel spec to the user kernelspec dir + // immediately so that Quarto/Jupyter can discover ggsql + // even if no session is ever started. The bundled kernel + // additionally needs this on every extension update, or the + // spec keeps pointing into the removed extension directory. + if (candidate.source === 'System' || candidate.source === 'Bundled') { writeKernelJson(getUserJupyterKernelDir(), candidate.kernelPath); } @@ -391,7 +552,7 @@ export class GgsqlRuntimeManager implements positron.LanguageRuntimeManager { log(`Yielding runtime: ${metadata.runtimeName} (${metadata.runtimeId}) at ${candidate.kernelPath}`); yield metadata; } else { - log(`Skipping inaccessible kernel: ${candidate.kernelPath}`); + log(`Skipping inaccessible kernel (${candidate.source}): ${candidate.kernelPath}`); } } diff --git a/ggsql-vscode/src/test/kernelDiscovery.test.ts b/ggsql-vscode/src/test/kernelDiscovery.test.ts new file mode 100644 index 00000000..427e1f99 --- /dev/null +++ b/ggsql-vscode/src/test/kernelDiscovery.test.ts @@ -0,0 +1,294 @@ +import * as assert from 'assert'; +import * as fs from 'fs'; +import * as os from 'os'; +import * as path from 'path'; +import * as vscode from 'vscode'; +import { + discoverKernelPaths, + generateMetadata, + isKernelAccessible, + resolveKernelStrategy, + selectKernelCandidates, + type KernelCandidate, +} from '../manager'; + +const EXTENSION_ID = 'ggsql.ggsql'; + +// Directories created by the helpers below, removed in suiteTeardown. +const tempDirs: string[] = []; + +function tempDir(): string { + const dir = fs.mkdtempSync(path.join(os.tmpdir(), 'ggsql-kernel-')); + tempDirs.push(dir); + return dir; +} + +const binaryName = process.platform === 'win32' ? 'ggsql-jupyter.exe' : 'ggsql-jupyter'; + +/** + * Build an extension directory that looks like an installed platform VSIX, + * with a stand-in for the kernel at bundled/bin/. + */ +function extensionDirWithBundle(mode = 0o755): { extensionPath: string; kernelPath: string } { + const extensionPath = tempDir(); + const binDir = path.join(extensionPath, 'bundled', 'bin'); + fs.mkdirSync(binDir, { recursive: true }); + const kernelPath = path.join(binDir, binaryName); + fs.writeFileSync(kernelPath, '#!/bin/sh\nexit 0\n', { mode }); + fs.chmodSync(kernelPath, mode); + return { extensionPath, kernelPath }; +} + +function contextFor(extensionPath: string): vscode.ExtensionContext { + return { extensionPath } as vscode.ExtensionContext; +} + +/** + * A WorkspaceConfiguration stub covering the two members + * resolveKernelStrategy() uses. Using a stub rather than writing real settings + * keeps the migration cases independent of the test instance's config state. + */ +function fakeConfig(values: { + strategy?: { global?: string; workspace?: string; workspaceFolder?: string }; + kernelPath?: string; +}): vscode.WorkspaceConfiguration { + return { + get: (key: string, defaultValue?: unknown) => + key === 'kernelPath' ? (values.kernelPath ?? defaultValue) : defaultValue, + inspect: (key: string) => + key === 'kernelStrategy' + ? { + key: 'ggsql.kernelStrategy', + defaultValue: 'bundled', + globalValue: values.strategy?.global, + workspaceValue: values.strategy?.workspace, + workspaceFolderValue: values.strategy?.workspaceFolder, + } + : undefined, + } as unknown as vscode.WorkspaceConfiguration; +} + +const HOST: KernelCandidate[] = [ + { kernelPath: '/usr/local/bin/ggsql-jupyter', source: 'System' }, + { kernelPath: '/opt/homebrew/bin/ggsql-jupyter', source: 'Path' }, +]; +const hostKernels = () => HOST; +const noHostKernels = () => []; + +suite('kernel strategy', () => { + test('the manifest declares bundled as the default', () => { + // The rest of the suite assumes this default; it is also what makes the + // extension work with no kernel installed. + const extension = vscode.extensions.getExtension(EXTENSION_ID); + const property = extension?.packageJSON.contributes.configuration.properties['ggsql.kernelStrategy']; + assert.ok(property, 'ggsql.kernelStrategy is not contributed'); + assert.strictEqual(property.default, 'bundled'); + assert.deepStrictEqual(property.enum, ['bundled', 'environment', 'path']); + }); + + test('an unset strategy resolves to bundled', () => { + assert.strictEqual(resolveKernelStrategy(fakeConfig({})), 'bundled'); + }); + + test('a configured kernelPath still means path', () => { + // Migration: users who set ggsql.kernelPath before the strategy setting + // existed must keep getting the kernel they named. + assert.strictEqual( + resolveKernelStrategy(fakeConfig({ kernelPath: '/opt/ggsql/ggsql-jupyter' })), + 'path', + ); + }); + + test('a whitespace-only kernelPath does not imply path', () => { + assert.strictEqual(resolveKernelStrategy(fakeConfig({ kernelPath: ' ' })), 'bundled'); + }); + + test('an explicit strategy overrides a configured kernelPath', () => { + // Otherwise a user could never keep a path around while asking for the + // bundled kernel. + const config = fakeConfig({ + strategy: { global: 'bundled' }, + kernelPath: '/opt/ggsql/ggsql-jupyter', + }); + assert.strictEqual(resolveKernelStrategy(config), 'bundled'); + }); + + test('workspace scope wins over global scope', () => { + const config = fakeConfig({ strategy: { global: 'environment', workspace: 'bundled' } }); + assert.strictEqual(resolveKernelStrategy(config), 'bundled'); + }); + + test('workspace folder scope wins over workspace scope', () => { + const config = fakeConfig({ strategy: { workspace: 'bundled', workspaceFolder: 'environment' } }); + assert.strictEqual(resolveKernelStrategy(config), 'environment'); + }); + + test('an unknown strategy falls back to bundled', () => { + // A hand-edited settings.json is not validated before it reaches here. + const config = fakeConfig({ strategy: { global: 'whatever' } }); + assert.strictEqual(resolveKernelStrategy(config), 'bundled'); + }); +}); + +suite('kernel candidate selection', () => { + const bundled = '/ext/ggsql.ggsql-0.5.0-darwin-arm64/bundled/bin/ggsql-jupyter'; + + test('bundled uses only the bundled kernel', () => { + const candidates = selectKernelCandidates('bundled', bundled, undefined, hostKernels); + assert.deepStrictEqual(candidates, [{ kernelPath: bundled, source: 'Bundled' }]); + }); + + test('bundled falls back to host kernels when the build carries none', () => { + // The platform-neutral VSIX ships no kernel and must keep working the + // way it does today. + const candidates = selectKernelCandidates('bundled', undefined, undefined, hostKernels); + assert.deepStrictEqual(candidates, HOST); + }); + + test('no bundle and no host kernel yields no candidates', () => { + // Regression test for the phantom runtime: discovery used to append the + // bare binary name unconditionally, so Positron registered a ggsql + // runtime that failed at session start with KS-19. + const candidates = selectKernelCandidates('bundled', undefined, undefined, noHostKernels); + assert.deepStrictEqual(candidates, []); + }); + + test('environment puts host kernels ahead of the bundled one', () => { + const candidates = selectKernelCandidates('environment', bundled, undefined, hostKernels); + assert.deepStrictEqual(candidates, [...HOST, { kernelPath: bundled, source: 'Bundled' }]); + }); + + test('environment falls back to the bundled kernel', () => { + const candidates = selectKernelCandidates('environment', bundled, undefined, noHostKernels); + assert.deepStrictEqual(candidates, [{ kernelPath: bundled, source: 'Bundled' }]); + }); + + test('path uses the configured kernel alone', () => { + // Neither the bundled kernel nor a host install may quietly stand in for + // the one the user named. + const candidates = selectKernelCandidates('path', bundled, '/opt/ggsql/ggsql-jupyter', hostKernels); + assert.deepStrictEqual(candidates, [ + { kernelPath: '/opt/ggsql/ggsql-jupyter', source: 'Setting' }, + ]); + }); + + test('path with no configured kernel behaves as bundled', () => { + const candidates = selectKernelCandidates('path', bundled, undefined, hostKernels); + assert.deepStrictEqual(candidates, [{ kernelPath: bundled, source: 'Bundled' }]); + }); +}); + +suite('kernel accessibility', () => { + test('a bare binary name is not accessible', async () => { + // Anything non-absolute reaching this check means the PATH lookup + // failed; accepting it is the other half of the phantom runtime. + assert.strictEqual(await isKernelAccessible(binaryName), false); + }); + + test('an executable file is accessible', async () => { + const { kernelPath } = extensionDirWithBundle(); + assert.strictEqual(await isKernelAccessible(kernelPath), true); + }); + + test('a missing file is not accessible', async () => { + assert.strictEqual(await isKernelAccessible(path.join(tempDir(), binaryName)), false); + }); + + test('a directory is not accessible', async () => { + // Directories carry the executable bit on POSIX, so an access() check + // on its own would pass one. + assert.strictEqual(await isKernelAccessible(tempDir()), false); + }); +}); + +suite('bundled kernel discovery', () => { + test('the bundled kernel is the only candidate under the default strategy', () => { + const { extensionPath, kernelPath } = extensionDirWithBundle(); + const candidates = discoverKernelPaths(contextFor(extensionPath)); + assert.deepStrictEqual(candidates, [{ kernelPath, source: 'Bundled' }]); + }); + + test('discovery never returns a path that is not on disk', () => { + // With no bundle, discovery falls through to the host locations. What is + // installed on the test machine is unknown, but every candidate it + // reports has to be a real absolute path. + const candidates = discoverKernelPaths(contextFor(tempDir())); + for (const candidate of candidates) { + assert.notStrictEqual(candidate.source, 'Bundled'); + assert.ok(path.isAbsolute(candidate.kernelPath), `${candidate.kernelPath} is not absolute`); + assert.ok(fs.existsSync(candidate.kernelPath), `${candidate.kernelPath} does not exist`); + } + }); + + test('a bundled kernel missing its executable bit is repaired', function () { + // Insurance against an unpack that drops the bit: without the repair the + // binary would be dropped as inaccessible and no runtime would appear. + if (process.platform === 'win32') { + this.skip(); + } + const { extensionPath, kernelPath } = extensionDirWithBundle(0o644); + const candidates = discoverKernelPaths(contextFor(extensionPath)); + assert.deepStrictEqual(candidates, [{ kernelPath, source: 'Bundled' }]); + assert.ok(fs.statSync(kernelPath).mode & 0o111, 'the executable bit was not restored'); + }); +}); + +suite('runtime metadata', () => { + // generateMetadata reads resources/ggsql-icon.svg from the extension folder, + // so these use the real one with a stand-in version. + function realContext(): vscode.ExtensionContext { + const extension = vscode.extensions.getExtension(EXTENSION_ID); + assert.ok(extension, `extension ${EXTENSION_ID} not found`); + return { + extensionPath: extension.extensionPath, + extension: { packageJSON: { version: '9.9.9' } }, + } as unknown as vscode.ExtensionContext; + } + + test('the bundled runtime id survives an extension update', () => { + // The bundled kernel lives under the versioned extension directory. An + // id derived from that path would change on every update, dropping the + // workspace's runtime affinity and its restorable sessions. + const context = realContext(); + const before = generateMetadata(context, { + kernelPath: '/ext/ggsql.ggsql-0.5.0-darwin-arm64/bundled/bin/ggsql-jupyter', + source: 'Bundled', + }); + const after = generateMetadata(context, { + kernelPath: '/ext/ggsql.ggsql-0.6.0-darwin-arm64/bundled/bin/ggsql-jupyter', + source: 'Bundled', + }); + assert.strictEqual(before.runtimeId, after.runtimeId); + }); + + test('the bundled runtime is named plain ggsql', () => { + // It is the default, so there is nothing to distinguish it from. + const metadata = generateMetadata(realContext(), { + kernelPath: '/ext/ggsql.ggsql-0.5.0/bundled/bin/ggsql-jupyter', + source: 'Bundled', + }); + assert.strictEqual(metadata.runtimeName, 'ggsql'); + }); + + test('other runtimes keep a per-path id and a qualified name', () => { + const context = realContext(); + const system = generateMetadata(context, { + kernelPath: '/usr/local/bin/ggsql-jupyter', + source: 'System', + }); + const setting = generateMetadata(context, { + kernelPath: '/opt/ggsql/ggsql-jupyter', + source: 'Setting', + }); + assert.strictEqual(system.runtimeName, 'ggsql (System)'); + assert.strictEqual(setting.runtimeName, 'ggsql (Setting)'); + assert.notStrictEqual(system.runtimeId, setting.runtimeId); + assert.notStrictEqual(system.runtimeId, 'ggsql-bundled'); + }); +}); + +suiteTeardown(() => { + for (const dir of tempDirs) { + fs.rmSync(dir, { recursive: true, force: true }); + } +}); From 1a6a9baac369a9b4f933d3b10014c55e983dd7a1 Mon Sep 17 00:00:00 2001 From: Sam Clark Date: Tue, 18 Aug 2026 10:33:13 -0500 Subject: [PATCH 2/8] Build and publish per-platform VSIXes carrying the kernel Each platform job now uploads its ggsql-jupyter binary as an artifact, taken after signing so the extension ships the same binary the installer does. A build-vsix job stages that artifact into ggsql-vscode/bundled/bin and packages one VSIX per target, plus a kernel-less universal build for platforms without one; publish-openvsx then publishes the packaged files. The VSIX build has to live in release-packages.yml rather than its own workflow: Actions artifacts are scoped to a workflow run, and two workflows triggered by the same tag run in parallel, so a separate workflow could not reach the kernels. Building in one run also keeps the kernel and the extension on the same commit. Open VSX takes the platform from the TargetPlatform attribute that vsce package --target writes into extension.vsixmanifest, so the packaged file is published as-is with no target passed to the publish action. create-release now names an artifact directory per glob instead of matching an extension anywhere under artifacts/, which would have swept the raw ggsql-jupyter.exe onto the release page alongside the installers. win32-arm64 is not built: no runner produces that kernel yet. release-vscode.yaml loses its tag trigger, which would otherwise race the new publish, and is left as a manual path for the universal VSIX alone. Co-Authored-By: Claude Opus 5 (1M context) --- .github/workflows/release-packages.yml | 207 ++++++++++++++++++++++++- .github/workflows/release-vscode.yaml | 15 +- CHANGELOG.md | 15 ++ ggsql-vscode/CHANGELOG.md | 7 + ggsql-vscode/CLAUDE.md | 11 ++ 5 files changed, 245 insertions(+), 10 deletions(-) diff --git a/.github/workflows/release-packages.yml b/.github/workflows/release-packages.yml index a463184f..19fc171e 100644 --- a/.github/workflows/release-packages.yml +++ b/.github/workflows/release-packages.yml @@ -52,6 +52,16 @@ jobs: SM_CLIENT_CERT_PASSWORD: ${{ secrets.SM_CLIENT_CERT_PASSWORD }} SM_CLIENT_CERT_FINGERPRINT: ${{ secrets.SM_CLIENT_CERT_FINGERPRINT }} + - name: Upload ggsql-jupyter kernel (win32-x64) + # Consumed by the build-vsix job, which bundles it into the per-platform + # VSIX. Uploaded after signing and before installer packaging, so the + # extension ships the same binary the installers do. + uses: actions/upload-artifact@v4 + with: + name: ggsql-jupyter-win32-x64 + path: target/release/ggsql-jupyter.exe + retention-days: 30 + - name: Build NSIS installer run: cargo packager --release --formats nsis @@ -153,6 +163,18 @@ jobs: --entitlements entitlements.plist \ --sign "$SIGN_ID" target/release/ggsql-jupyter + - name: Upload ggsql-jupyter kernel (darwin-x64) + # Consumed by the build-vsix job, which bundles it into the per-platform + # VSIX. Uploaded after signing and before installer packaging, so the + # extension ships the same signed binary the installers do. The Mach-O + # signature is embedded in the file, so it survives the artifact zip; + # the executable bit does not, and build-vsix restores it. + uses: actions/upload-artifact@v4 + with: + name: ggsql-jupyter-darwin-x64 + path: target/release/ggsql-jupyter + retention-days: 30 + - name: Build and notarize PKG installer (x86_64) # NOTE: --sign uses the Developer ID *Installer* cert (signs .pkg only), # distinct from the Developer ID Application cert used to sign Mach-O above. @@ -260,6 +282,14 @@ jobs: --entitlements entitlements.plist \ --sign "$SIGN_ID" target/release/ggsql-jupyter + - name: Upload ggsql-jupyter kernel (darwin-arm64) + # See the darwin-x64 job for why this sits between signing and packaging. + uses: actions/upload-artifact@v4 + with: + name: ggsql-jupyter-darwin-arm64 + path: target/release/ggsql-jupyter + retention-days: 30 + - name: Build and notarize PKG installer (aarch64) # NOTE: --sign uses the Developer ID *Installer* cert (signs .pkg only), # distinct from the Developer ID Application cert used to sign Mach-O above. @@ -337,6 +367,15 @@ jobs: - name: Build ggsql binary (x86_64) run: cargo build --release --bin ggsql --bin ggsql-jupyter + - name: Upload ggsql-jupyter kernel (linux-x64) + # Consumed by the build-vsix job, which bundles it into the per-platform + # VSIX and restores the executable bit the artifact zip drops. + uses: actions/upload-artifact@v4 + with: + name: ggsql-jupyter-linux-x64 + path: target/release/ggsql-jupyter + retention-days: 30 + - name: Build Debian package (x86_64) run: cargo packager --release --formats deb @@ -384,6 +423,14 @@ jobs: - name: Build ggsql binary (aarch64) run: cargo build --release --bin ggsql --bin ggsql-jupyter + - name: Upload ggsql-jupyter kernel (linux-arm64) + # See the linux-x64 job. + uses: actions/upload-artifact@v4 + with: + name: ggsql-jupyter-linux-arm64 + path: target/release/ggsql-jupyter + retention-days: 30 + - name: Build Debian package (aarch64) run: cargo packager --release --formats deb @@ -545,6 +592,150 @@ jobs: - name: Publish to npm run: npm publish ./npm-tarball/*.tgz --access=public --provenance --tag ${{ steps.dist-tag.outputs.tag }} + build-vsix: + name: Build VSIX (${{ matrix.target }}) + # This lives here rather than in its own workflow because the kernel + # binaries are artifacts scoped to a single workflow run: a separate + # workflow triggered by the same tag could not download them. Building the + # VSIX in the same run also means the bundled kernel and the extension can + # never come from different commits. + needs: [build-windows, build-macos-x86_64, build-macos-aarch64, build-linux-x86_64, build-linux-aarch64] + runs-on: ubuntu-latest + permissions: + contents: read + strategy: + fail-fast: false + matrix: + # win32-arm64 is deliberately absent: no runner builds that kernel yet. + # "universal" carries no kernel and is what users on any other platform + # install, alongside the kernel from a native installer. + target: + - darwin-arm64 + - darwin-x64 + - linux-arm64 + - linux-x64 + - win32-x64 + - universal + + steps: + - name: Checkout code + uses: actions/checkout@v4 + + - name: Install Node.js + uses: actions/setup-node@v4 + with: + node-version: "22" + cache: npm + cache-dependency-path: ggsql-vscode/package-lock.json + + - name: Install vsce + run: npm install -g @vscode/vsce + + - name: Install dependencies + working-directory: ggsql-vscode + run: npm ci + + - name: Download ggsql-jupyter kernel (${{ matrix.target }}) + if: matrix.target != 'universal' + uses: actions/download-artifact@v4 + with: + name: ggsql-jupyter-${{ matrix.target }} + path: ggsql-vscode/bundled/bin + + - name: Make the kernel executable + # Artifact upload and download do not preserve the executable bit. The + # extension repairs it at runtime too, but it has to be right inside the + # VSIX for a fresh install to start a session. + if: matrix.target != 'universal' + run: | + chmod +x ggsql-vscode/bundled/bin/* + ls -l ggsql-vscode/bundled/bin + + - name: Package VSIX + id: package + working-directory: ggsql-vscode + env: + TARGET: ${{ matrix.target }} + run: | + VERSION="$(node -p 'require("./package.json").version')" + VSIX="ggsql-${VERSION}-${TARGET}.vsix" + if [ "$TARGET" = universal ]; then + vsce package --out "$VSIX" + else + # --target writes TargetPlatform into the vsixmanifest, which is what + # Open VSX records and what Positron's bootstrap asks for by name. + vsce package --target "$TARGET" --out "$VSIX" + fi + echo "vsix=$VSIX" >> "$GITHUB_OUTPUT" + + - name: Check the VSIX contents + working-directory: ggsql-vscode + env: + TARGET: ${{ matrix.target }} + VSIX: ${{ steps.package.outputs.vsix }} + run: | + unzip -l "$VSIX" + unzip -p "$VSIX" extension.vsixmanifest \ + | grep -o 'TargetPlatform="[^"]*"' || echo 'no TargetPlatform: universal' + if [ "$TARGET" = universal ]; then + if unzip -l "$VSIX" | grep -q 'extension/bundled/'; then + echo "::error::the universal VSIX must not carry a kernel" + exit 1 + fi + elif ! unzip -l "$VSIX" | grep -q 'extension/bundled/bin/ggsql-jupyter'; then + echo "::error::the $TARGET VSIX is missing its bundled kernel" + exit 1 + fi + + - name: Upload VSIX + uses: actions/upload-artifact@v4 + with: + name: ggsql-vsix-${{ matrix.target }} + path: ggsql-vscode/${{ steps.package.outputs.vsix }} + retention-days: 30 + + publish-openvsx: + name: Publish VSIX (${{ matrix.target }}) + # Separate from build-vsix so that a registry failure neither blocks the + # GitHub release nor forces the VSIXes to be rebuilt on a retry. + needs: [build-vsix] + runs-on: ubuntu-latest + if: startsWith(github.ref, 'refs/tags/v') + permissions: + contents: read + strategy: + fail-fast: false + matrix: + target: + - darwin-arm64 + - darwin-x64 + - linux-arm64 + - linux-x64 + - win32-x64 + - universal + + steps: + - name: Download VSIX + uses: actions/download-artifact@v4 + with: + name: ggsql-vsix-${{ matrix.target }} + path: vsix + + - name: Locate the VSIX + id: vsix + run: echo "path=$(ls vsix/*.vsix)" >> "$GITHUB_OUTPUT" + + - name: Publish to Open VSX Registry + # The packaged file is published rather than a target being passed here: + # Open VSX reads the platform from TargetPlatform in the vsixmanifest + # that `vsce package --target` wrote, and ovsx discards a target option + # when it is handed an already-packaged vsix. + uses: HaaLeo/publish-vscode-extension@v2 + with: + pat: ${{ secrets.OPEN_VSX_TOKEN }} + skipDuplicate: true + extensionFile: ${{ steps.vsix.outputs.path }} + create-release: name: Create GitHub Release needs: [build-windows, build-macos-x86_64, build-macos-aarch64, build-linux-x86_64, build-linux-aarch64, build-cargo, build-wasm] @@ -565,11 +756,17 @@ jobs: - name: Create release and upload installers uses: softprops/action-gh-release@v2 with: + # Scoped to one artifact directory each, rather than matching an + # extension anywhere under artifacts/. Release assets are the only + # anonymously downloadable output of this workflow, so what lands there + # is named explicitly: an `artifacts/**/*.exe` glob would also sweep up + # the raw ggsql-jupyter.exe that build-vsix consumes, publishing one + # platform's bare kernel next to the installers. files: | - artifacts/**/*.exe - artifacts/**/*.msi - artifacts/**/*.pkg - artifacts/**/*.deb - artifacts/**/*.tgz + artifacts/ggsql-windows-nsis/*.exe + artifacts/ggsql-windows-msi/*.msi + artifacts/ggsql-macos-pkg-*/*.pkg + artifacts/ggsql-linux-deb-*/*.deb + artifacts/ggsql-wasm-npm/*.tgz env: GITHUB_TOKEN: ${{ secrets.GITHUB_TOKEN }} diff --git a/.github/workflows/release-vscode.yaml b/.github/workflows/release-vscode.yaml index c14d5f0e..cf50704a 100644 --- a/.github/workflows/release-vscode.yaml +++ b/.github/workflows/release-vscode.yaml @@ -1,9 +1,14 @@ -name: Open VSX Release - +name: Open VSX Release (manual, universal only) + +# Releases are driven by release-packages.yml: its build-vsix and +# publish-openvsx jobs build and publish all six VSIXes, five of which carry a +# bundled ggsql-jupyter kernel downloaded from the platform build jobs in the +# same run. This workflow is kept only as a manual path for republishing the +# kernel-less universal VSIX on its own, and deliberately has no tag trigger: +# on a tag it would race the publish in release-packages.yml. Dispatch it +# against a tag rather than a branch — the publish step is still gated on a tag +# ref, so a dispatch from a branch packages the VSIX and stops there. on: - push: - tags: - - "v*" workflow_dispatch: permissions: diff --git a/CHANGELOG.md b/CHANGELOG.md index 5ec8c0c1..e4d96cda 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -37,6 +37,16 @@ transformation. This has no Vega-Lite equivalent and is ignored by that writer; the png writer draws them. +- The VS Code / Positron extension now ships the `ggsql-jupyter` kernel, so + installing the extension is all that is needed to run queries — no separate + native install. The per-platform builds (`darwin-arm64`, `darwin-x64`, + `linux-arm64`, `linux-x64`, `win32-x64`) each carry the same signed kernel + binary the matching installer does, and a kernel-less universal build remains + for any other platform, where the installer is still required. A new + `ggsql.kernelStrategy` setting picks between the bundled kernel (the default), + a kernel installed on the machine, and a fixed path in `ggsql.kernelPath`; + configuring `ggsql.kernelPath` alone continues to mean that path is used. + ### Changed - Dodging now only takes effect where groups actually meet on a position. A layer whose grouping gives every group a position of its own — `colour` mapped @@ -64,6 +74,11 @@ category ticks pulled toward the middle of the panel. ### Fixed +- Positron no longer offers a ggsql runtime on a machine that has no kernel. + Discovery added `ggsql-jupyter` as a candidate whether or not it was on `PATH`, + and the accessibility check waved through any bare name, so starting the + console failed with `KS-19: Kernel path not found` instead of ggsql simply not + being listed. - A dodged violin or half-boxplot on a categorical `y` axis is no longer flipped in the Vega-Lite writer. Both took their band displacement from an encoding of their own that read a ggsql offset as pointing down the screen, so their groups diff --git a/ggsql-vscode/CHANGELOG.md b/ggsql-vscode/CHANGELOG.md index bc8208d0..eaa6ab97 100644 --- a/ggsql-vscode/CHANGELOG.md +++ b/ggsql-vscode/CHANGELOG.md @@ -2,6 +2,13 @@ ## [Unreleased] +- The extension now ships the `ggsql-jupyter` kernel, so installing it is enough + to run queries in Positron. `ggsql.kernelStrategy` picks between the bundled + kernel (the default), a kernel installed on this machine, and a fixed path in + `ggsql.kernelPath`. +- Fixed: no ggsql runtime is offered when no kernel can be found, rather than one + that fails at session start with `KS-19: Kernel path not found`. + ## 0.3.2 - Improved configuration options shown in Positron Connection pane for sqlite diff --git a/ggsql-vscode/CLAUDE.md b/ggsql-vscode/CLAUDE.md index db3cf0e0..bf14d7cb 100644 --- a/ggsql-vscode/CLAUDE.md +++ b/ggsql-vscode/CLAUDE.md @@ -153,6 +153,17 @@ npx vsce package # produces ggsql-.vsix code --install-extension ggsql-.vsix ``` +A local `vsce package` produces the kernel-less VSIX, since `bundled/` only exists in a release build. + +**Release builds** live in [`/.github/workflows/release-packages.yml`](../.github/workflows/release-packages.yml), not in a workflow of their own. Its `build-vsix` job runs a matrix of six — the five platform targets plus `universal` — downloading the `ggsql-jupyter-` artifact each platform job uploaded between signing and installer packaging, restoring the executable bit, and running `vsce package --target `. `publish-openvsx` then publishes the packaged file to Open VSX. + +Four things about that arrangement are deliberate: + +- **The VSIX build cannot live in its own workflow.** Actions artifacts are scoped to a single workflow run, and two workflows triggered by the same tag run in parallel, so a separate workflow could not download the kernels. Building in the same run also means the kernel and the extension always come from one commit. +- **The executable bit has to be restored after download.** Artifact upload and download drop it. It does survive `vsce package` into the VSIX itself, so restoring it once in CI is enough; `ensureExecutable()` in `manager.ts` is belt-and-braces for an install that loses it. +- **The published artefact is the packaged `.vsix`, with no `target` passed to the publish action.** Open VSX reads the platform from the `TargetPlatform` attribute that `vsce package --target` writes into `extension.vsixmanifest`, and defaults to `universal` when it is absent; `ovsx` discards a target option when handed an already-packaged vsix. +- **`win32-arm64` is not built.** No runner produces that kernel yet. Positron's bootstrap appends `?targetPlatform=` and gets an HTTP 403 rather than the universal build for a target that was never published, so the universal VSIX is not a fallback for it — see posit-dev/positron#14954. + 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. From 085e2df3d718b3b6fe7c0825c52ec5a7a77adc18 Mon Sep 17 00:00:00 2001 From: Sam Clark Date: Tue, 18 Aug 2026 10:54:22 -0500 Subject: [PATCH 3/8] Removed separate VSCode build job --- .github/workflows/release-vscode.yaml | 47 --------------------------- 1 file changed, 47 deletions(-) delete mode 100644 .github/workflows/release-vscode.yaml diff --git a/.github/workflows/release-vscode.yaml b/.github/workflows/release-vscode.yaml deleted file mode 100644 index cf50704a..00000000 --- a/.github/workflows/release-vscode.yaml +++ /dev/null @@ -1,47 +0,0 @@ -name: Open VSX Release (manual, universal only) - -# Releases are driven by release-packages.yml: its build-vsix and -# publish-openvsx jobs build and publish all six VSIXes, five of which carry a -# bundled ggsql-jupyter kernel downloaded from the platform build jobs in the -# same run. This workflow is kept only as a manual path for republishing the -# kernel-less universal VSIX on its own, and deliberately has no tag trigger: -# on a tag it would race the publish in release-packages.yml. Dispatch it -# against a tag rather than a branch — the publish step is still gated on a tag -# ref, so a dispatch from a branch packages the VSIX and stops there. -on: - workflow_dispatch: - -permissions: - contents: read - -jobs: - build: - runs-on: ubuntu-latest - - steps: - - name: Check out repository - uses: actions/checkout@v4 - - - name: Setup Node.js - uses: actions/setup-node@v4 - with: - node-version: "22" - - - name: Install vsce - run: npm install -g @vscode/vsce - - - name: Install dependencies - working-directory: ggsql-vscode - run: npm ci - - - name: Package VSIX - working-directory: ggsql-vscode - run: vsce package - - - name: Publish to Open VSX Registry - if: startsWith(github.ref, 'refs/tags/v') - uses: HaaLeo/publish-vscode-extension@v2 - with: - pat: ${{ secrets.OPEN_VSX_TOKEN }} - skipDuplicate: true - packagePath: ggsql-vscode From 833cf8999e4beaa146755171b49411d4cdd4868a Mon Sep 17 00:00:00 2001 From: Sam Clark Date: Tue, 18 Aug 2026 11:54:03 -0500 Subject: [PATCH 4/8] Close the gaps in kernel-discovery test coverage Adds the layer that was missing: a Positron integration suite that launches the bundled kernel. The unit suites cover discovery precedence and the release workflow proves the binary is inside the VSIX, but nothing until now started the binary, which is the failure the bundling work is about. It asserts one registered ggsql runtime with the ggsql-bundled id, and that executeCode starts a session and returns a result. Session creation needs positron.positron-supervisor, so the harness runs with disableExtensions: false; the suite drives mocha itself because extensionTestsPath must export run(). .vscode-test.mjs now globs one level so the Positron suite does not also run under stock VS Code, where it cannot pass. discoverAllRuntimes was untested, so the "no kernel means no runtime" requirement was only checked one level down at selectKernelCandidates, which returns candidates rather than runtimes. Testing it needed a seam: discovery writes a Jupyter kernel spec as a side effect, and with the default directory a test run would repoint the real kernelspec at a fixture. GgsqlRuntimeManager therefore takes an optional kernelSpecDir. Also covered: host discovery and the symlink dedupe, by redirecting HOME and PATH rather than depending on what the developer has installed; the strategy settings read through the real configuration service, since a stubbed inspect() cannot prove the migration; and resolveConfiguredPath. The old fallback test passed vacuously on any machine without a kernel installed, which was every machine including CI. test-extension.yaml gains a three-OS matrix, because discovery branches on the OS for the binary name, the PATH lookup, the executable-bit repair and the locations it searches, and a packaging job asserting the universal VSIX stays kernel-less on every PR rather than only at release time. Co-Authored-By: Claude Opus 5 (1M context) --- .github/workflows/test-extension.yaml | 122 ++++++- ggsql-vscode/.gitignore | 2 + ggsql-vscode/.vscode-test.mjs | 5 +- ggsql-vscode/CLAUDE.md | 33 +- ggsql-vscode/package-lock.json | 33 ++ ggsql-vscode/package.json | 2 + ggsql-vscode/src/manager.ts | 24 +- .../src/test/integration/console.test.ts | 100 ++++++ ggsql-vscode/src/test/integration/index.ts | 42 +++ ggsql-vscode/src/test/kernelDiscovery.test.ts | 332 ++++++++++++++++-- ggsql-vscode/src/test/runIntegration.ts | 50 +++ 11 files changed, 696 insertions(+), 49 deletions(-) create mode 100644 ggsql-vscode/src/test/integration/console.test.ts create mode 100644 ggsql-vscode/src/test/integration/index.ts create mode 100644 ggsql-vscode/src/test/runIntegration.ts diff --git a/.github/workflows/test-extension.yaml b/.github/workflows/test-extension.yaml index 2816e727..71130bac 100644 --- a/.github/workflows/test-extension.yaml +++ b/.github/workflows/test-extension.yaml @@ -8,14 +8,23 @@ on: workflow_dispatch: jobs: - # Runs the suite against stock VS Code. A sibling job will run the same - # extension against Positron, which covers the language runtime, connection - # drivers and cell execution that stock VS Code cannot reach. + # Runs the suite against stock VS Code, on all three platforms the extension + # ships a bundled kernel for. The platform matrix is not incidental: kernel + # discovery branches on the OS for the binary name, the PATH lookup + # (which/where), the executable-bit repair, and the install locations it + # searches. test-extension: - runs-on: ubuntu-latest - name: Test (VS Code) + runs-on: ${{ matrix.os }} + name: Test (VS Code, ${{ matrix.os }}) + strategy: + fail-fast: false + matrix: + os: [ubuntu-latest, macos-latest, windows-latest] defaults: run: + # bash on every runner, so the npm scripts and their globs behave + # identically rather than going through PowerShell on Windows. + shell: bash working-directory: ggsql-vscode steps: @@ -32,13 +41,116 @@ jobs: - name: Install XVFB # The extension tests drive a real VS Code instance, which needs a # display. The grammar tests do not, but run under the same command. + if: runner.os == 'Linux' run: sudo apt-get -y update && sudo apt-get -y install xvfb - name: Install dependencies run: npm ci - name: Lint + # Nothing platform-specific to check, so once is enough. + if: runner.os == 'Linux' run: npm run lint - name: Run tests + if: runner.os == 'Linux' run: xvfb-run -a npm test + + - name: Run tests + if: runner.os != 'Linux' + run: npm test + + # Packaging invariants the release workflow depends on. That workflow only + # runs on a tag, so without this a broken .vscodeignore or manifest would not + # surface until release time. No kernel is staged here: this is the + # kernel-less universal build, which must stay kernel-less. + test-packaging: + runs-on: ubuntu-latest + name: Package (universal VSIX) + defaults: + run: + working-directory: ggsql-vscode + + steps: + - name: Check out repository + uses: actions/checkout@v4 + + - name: Setup Node.js + uses: actions/setup-node@v4 + with: + node-version: "22" + cache: npm + cache-dependency-path: ggsql-vscode/package-lock.json + + - name: Install dependencies + run: npm ci + + - name: Package VSIX + run: npx --yes @vscode/vsce package --out ggsql-universal.vsix + + - name: Check the VSIX contents + run: | + unzip -l ggsql-universal.vsix + if unzip -l ggsql-universal.vsix | grep -q 'extension/bundled/'; then + echo "::error::the universal VSIX must not carry a kernel" + exit 1 + fi + for required in extension/out/extension.js extension/syntaxes/ggsql.tmLanguage.json extension/resources/ggsql-icon.svg; do + if ! unzip -l ggsql-universal.vsix | grep -q "$required"; then + echo "::error::$required is missing from the VSIX" + exit 1 + fi + done + + # The one check that proves the bundled binary actually starts. The unit + # suites cover precedence and metadata, and the release workflow proves the + # binary is inside the VSIX; only this launches it, which is the failure the + # bundling work is about. + test-integration: + runs-on: ubuntu-latest + name: Integration (Positron + bundled kernel) + defaults: + run: + working-directory: ggsql-vscode + + steps: + - name: Check out repository + uses: actions/checkout@v4 + + - name: Setup Node.js + uses: actions/setup-node@v4 + with: + node-version: "22" + cache: npm + cache-dependency-path: ggsql-vscode/package-lock.json + + - name: Install Rust toolchain + uses: dtolnay/rust-toolchain@1.86.0 + + - name: Cache cargo build + # The kernel links DuckDB statically, so an uncached release build is + # the expensive part of this job by a wide margin. + uses: Swatinem/rust-cache@v2 + + - name: Install tree-sitter-cli + run: npm install -g tree-sitter-cli + + - name: Install XVFB + run: sudo apt-get -y update && sudo apt-get -y install xvfb + + - name: Build the kernel + working-directory: . + run: cargo build --release --bin ggsql-jupyter + + - name: Stage the kernel where the VSIX would carry it + working-directory: . + run: | + mkdir -p ggsql-vscode/bundled/bin + cp target/release/ggsql-jupyter ggsql-vscode/bundled/bin/ + chmod +x ggsql-vscode/bundled/bin/ggsql-jupyter + + - name: Install dependencies + run: npm ci + + - name: Run integration tests + run: xvfb-run -a npm run test:integration diff --git a/ggsql-vscode/.gitignore b/ggsql-vscode/.gitignore index 6f6ad1ae..d8d1593a 100644 --- a/ggsql-vscode/.gitignore +++ b/ggsql-vscode/.gitignore @@ -1,4 +1,6 @@ out out-test .vscode-test/ +.positron-test/ bundled +*.vsix diff --git a/ggsql-vscode/.vscode-test.mjs b/ggsql-vscode/.vscode-test.mjs index a6053b90..931dc9a0 100644 --- a/ggsql-vscode/.vscode-test.mjs +++ b/ggsql-vscode/.vscode-test.mjs @@ -1,6 +1,9 @@ import { defineConfig } from '@vscode/test-cli'; export default defineConfig({ - files: 'out-test/test/**/*.test.js', + // Only the suites directly under out-test/test/. test/integration/ is + // deliberately excluded: it needs a real Positron, which src/test/ + // runIntegration.ts downloads and launches instead (npm run test:integration). + files: 'out-test/test/*.test.js', mocha: { timeout: 5000 }, }); diff --git a/ggsql-vscode/CLAUDE.md b/ggsql-vscode/CLAUDE.md index bf14d7cb..c999ebe1 100644 --- a/ggsql-vscode/CLAUDE.md +++ b/ggsql-vscode/CLAUDE.md @@ -172,9 +172,10 @@ For an interactive session, open the **repo root** in Positron and press F5 ```sh cd ggsql-vscode -npm test # grammar scopes, then the VS Code suites -npm run test:grammar # TextMate scopes only; no Electron, fast +npm test # grammar scopes, then the VS Code suites +npm run test:grammar # TextMate scopes only; no Electron, fast npm run test:extension +npm run test:integration # downloads Positron; needs a staged kernel (see below) ``` Tests live in `src/test/` and compile to `out-test/` via `tsconfig.test.json`, deliberately not to `out/`, which `esbuild.js` owns. The whole of `src/` compiles there, not just `src/test/`, because the unit tests import the extension's own modules. `@vscode/test-cli` launches a real VS Code instance, so a window appears while the suites run; CI wraps the same command in `xvfb-run`. @@ -183,7 +184,33 @@ Note that `tsc` does not prune output for deleted sources: if you delete or rena The suites cover the extension as stock VS Code sees it: activation, language resolution, cell parsing, `.sql` gating, CodeLens placement, TextMate scopes, kernel discovery, 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. +Add new tests as `src/test/.test.ts`; no config change is needed. `.vscode-test.mjs` globs `out-test/test/*.test.js` — one level only, deliberately, so the Positron suite in `test/integration/` does not run under stock VS Code, where it cannot pass. + +### The Positron integration suite + +`src/test/integration/` is the only place a kernel is actually launched. The unit suites cover discovery precedence and metadata, and `build-vsix` proves the binary is inside the VSIX; neither can tell whether it *starts*, which is the failure the bundling work exists to fix. `npm run test:integration` builds nothing itself — stage a kernel first: + +```sh +cargo build --release --bin ggsql-jupyter +mkdir -p ggsql-vscode/bundled/bin && cp target/release/ggsql-jupyter ggsql-vscode/bundled/bin/ +``` + +`src/test/runIntegration.ts` then downloads Positron via [`@posit-dev/positron-test-electron`](https://github.com/posit-dev/positron-test-electron) and runs the suite in its extension host. Three details are load bearing: + +- **`disableExtensions: false`.** Session creation goes through `positron.positron-supervisor`, one of Positron's bundled extensions. Under the harness's default `--disable-extensions` there is no supervisor and every session start fails. +- **The suite drives mocha itself.** `extensionTestsPath` must resolve to a module exporting `run()`, which is why `test/integration/index.ts` exists instead of the `@vscode/test-cli` config the other suites use. Its timeout is 120s: a session start spawns the binary and completes a Jupyter handshake. +- **`channel: 'daily'`.** Positron's stable channel is not published for every platform. Pin `version` instead once a known-good build is worth freezing. + +The download is cached in `.positron-test/`, gitignored like `.vscode-test/`. It keeps a directory per Positron version, so it grows as dailies move on — around 3 GB after one run, and worth clearing occasionally rather than a leak to fix. + +The assertions worth keeping: exactly one ggsql runtime, its `runtimeId` is `ggsql-bundled` and its path is under `bundled/bin/`, and `executeCode` returns a result — which starts a session if none is running, so it covers spawn, handshake and execution in one call. + +### Testing discovery without wrecking the developer's machine + +Two seams exist because discovery reads and writes real state: + +- `GgsqlRuntimeManager` takes `{ kernelSpecDir }`. Discovery advertises the kernel by writing a Jupyter kernel spec, so a test that called `discoverAllRuntimes()` with the default would repoint the *real* kernelspec — the one Quarto resolves — at a temp fixture. +- `kernelDiscovery.test.ts` redirects `HOME`, `USERPROFILE`, `APPDATA`, `LOCALAPPDATA` and `PATH` to stage host kernels, restoring them in teardown. The native-installer locations (`/usr/local/bin`, `/usr/bin`, `/Applications`) are hard-coded absolutes that no environment variable can redirect, so the few tests needing "no kernel anywhere" call `systemInstallPresent()` and skip on a machine that has one. CI never does, which is where those regressions matter. ### Editing the grammar fixture diff --git a/ggsql-vscode/package-lock.json b/ggsql-vscode/package-lock.json index 8e8db899..f621c0de 100644 --- a/ggsql-vscode/package-lock.json +++ b/ggsql-vscode/package-lock.json @@ -13,6 +13,7 @@ }, "devDependencies": { "@posit-dev/positron": "^0.2.7", + "@posit-dev/positron-test-electron": "^0.0.3", "@types/mocha": "^10.0.10", "@types/node": "^18.x", "@types/vscode": "^1.75.0", @@ -765,6 +766,38 @@ "@types/vscode": "^1.74.0" } }, + "node_modules/@posit-dev/positron-test-electron": { + "version": "0.0.3", + "resolved": "https://registry.npmjs.org/@posit-dev/positron-test-electron/-/positron-test-electron-0.0.3.tgz", + "integrity": "sha512-MQYKCoB9JlGd70QLV0BVTezzGljIduLskeczDLVZ/4ECuYKAtuVFHvXFLDv5BUoarSxHkc0cT6r9k7DwrSni3A==", + "dev": true, + "dependencies": { + "@vscode/test-electron": "^2.4.1" + }, + "bin": { + "positron-test-electron": "out/cli.js" + }, + "engines": { + "node": ">=20" + } + }, + "node_modules/@posit-dev/positron-test-electron/node_modules/@vscode/test-electron": { + "version": "2.5.2", + "resolved": "https://registry.npmjs.org/@vscode/test-electron/-/test-electron-2.5.2.tgz", + "integrity": "sha512-8ukpxv4wYe0iWMRQU18jhzJOHkeGKbnw7xWRX3Zw1WJA4cEKbHcmmLPdPrPtL6rhDcrlCZN+xKRpv09n4gRHYg==", + "dev": true, + "license": "MIT", + "dependencies": { + "http-proxy-agent": "^7.0.2", + "https-proxy-agent": "^7.0.5", + "jszip": "^3.10.1", + "ora": "^8.1.0", + "semver": "^7.6.2" + }, + "engines": { + "node": ">=16" + } + }, "node_modules/@types/esrecurse": { "version": "4.3.1", "resolved": "https://registry.npmjs.org/@types/esrecurse/-/esrecurse-4.3.1.tgz", diff --git a/ggsql-vscode/package.json b/ggsql-vscode/package.json index 0df030d9..77d59ca1 100644 --- a/ggsql-vscode/package.json +++ b/ggsql-vscode/package.json @@ -200,6 +200,7 @@ "pretest": "npm run compile-tests && npm run package", "test:grammar": "vscode-tmgrammar-test -g syntaxes/ggsql.tmLanguage.json \"src/test/grammar/*.gsql\"", "test:extension": "vscode-test", + "test:integration": "npm run compile-tests && npm run package && node ./out-test/test/runIntegration.js", "test": "npm run test:grammar && npm run test:extension" }, "dependencies": { @@ -207,6 +208,7 @@ }, "devDependencies": { "@posit-dev/positron": "^0.2.7", + "@posit-dev/positron-test-electron": "^0.0.3", "@types/mocha": "^10.0.10", "@types/node": "^18.x", "@types/vscode": "^1.75.0", diff --git a/ggsql-vscode/src/manager.ts b/ggsql-vscode/src/manager.ts index 6f0f5262..b21cd7df 100644 --- a/ggsql-vscode/src/manager.ts +++ b/ggsql-vscode/src/manager.ts @@ -76,7 +76,7 @@ function findOnPath(binaryName: string): string | undefined { * as-is, so that discovery rejects it as inaccessible and logs it back to the * user rather than silently ignoring the setting. */ -function resolveConfiguredPath(configuredPath: string): string { +export function resolveConfiguredPath(configuredPath: string): string { if (path.isAbsolute(configuredPath)) { return configuredPath; } @@ -499,6 +499,21 @@ export async function getSupervisorApi(): Promise { return supervisorExt.activate(); } +/** + * Overrides for GgsqlRuntimeManager's environment. + */ +export interface RuntimeManagerOptions { + /** + * Directory the discovered kernel is advertised in, as a Jupyter kernel + * spec. Defaults to the user-level Jupyter kernels directory. + * + * Discovery writes that spec as a side effect, so tests point this at a + * temp directory: otherwise running discovery would repoint the real + * kernelspec — the one Quarto and Jupyter resolve — at a test fixture. + */ + kernelSpecDir?: string; +} + /** * ggsql Language Runtime Manager * @@ -517,9 +532,11 @@ export class GgsqlRuntimeManager implements positron.LanguageRuntimeManager { public readonly alwaysRediscover = true; private _context: vscode.ExtensionContext; + private _kernelSpecDir: string; - constructor(context: vscode.ExtensionContext) { + constructor(context: vscode.ExtensionContext, options: RuntimeManagerOptions = {}) { this._context = context; + this._kernelSpecDir = options.kernelSpecDir ?? getUserJupyterKernelDir(); } /** @@ -529,6 +546,7 @@ export class GgsqlRuntimeManager implements positron.LanguageRuntimeManager { */ discoverAllRuntimes(): AsyncGenerator { const context = this._context; + const kernelSpecDir = this._kernelSpecDir; const generator = async function* discoverGgsqlRuntimes() { log('Discovering ggsql runtimes...'); @@ -545,7 +563,7 @@ export class GgsqlRuntimeManager implements positron.LanguageRuntimeManager { // additionally needs this on every extension update, or the // spec keeps pointing into the removed extension directory. if (candidate.source === 'System' || candidate.source === 'Bundled') { - writeKernelJson(getUserJupyterKernelDir(), candidate.kernelPath); + writeKernelJson(kernelSpecDir, candidate.kernelPath); } const metadata = generateMetadata(context, candidate); diff --git a/ggsql-vscode/src/test/integration/console.test.ts b/ggsql-vscode/src/test/integration/console.test.ts new file mode 100644 index 00000000..1d7aa306 --- /dev/null +++ b/ggsql-vscode/src/test/integration/console.test.ts @@ -0,0 +1,100 @@ +/* + * End-to-end check that the kernel bundled in this extension actually runs. + * + * Everything else about bundling is verified without a kernel process: the unit + * suites assert precedence and metadata, and the release workflow asserts the + * binary is inside the VSIX. Neither can tell whether the binary starts. This + * suite does, which is the failure the whole change is about (`KS-19: Kernel + * path not found`, and its cousins — a wrong-architecture or unsigned binary + * that Positron cannot launch). + * + * Requires ggsql-vscode/bundled/bin/ggsql-jupyter to exist; CI builds it first. + */ + +import * as assert from 'assert'; +import * as path from 'path'; +import * as vscode from 'vscode'; +import type { PositronApi } from '@posit-dev/positron'; +import { getPositronApi } from '../../positronApi'; + +const EXTENSION_ID = 'ggsql.ggsql'; + +/** Poll until `probe` returns a value, or fail after `timeoutMs`. */ +async function waitFor(what: string, timeoutMs: number, probe: () => Promise): Promise { + const deadline = Date.now() + timeoutMs; + for (;;) { + const found = await probe(); + if (found !== undefined) { + return found; + } + if (Date.now() > deadline) { + throw new Error(`timed out after ${timeoutMs}ms waiting for ${what}`); + } + await new Promise(resolve => setTimeout(resolve, 500)); + } +} + +suite('bundled kernel in Positron', () => { + let positron: PositronApi; + + suiteSetup(async () => { + const extension = vscode.extensions.getExtension(EXTENSION_ID); + assert.ok(extension, `extension ${EXTENSION_ID} not found`); + await extension.activate(); + + const api = getPositronApi(); + assert.ok(api, 'no Positron API; this suite must run under Positron, not VS Code'); + positron = api; + }); + + test('the bundled kernel is registered as the only ggsql runtime', async () => { + // Discovery runs on window open, so the runtime may not be registered the + // instant activation returns. + const runtimes = await waitFor('a registered ggsql runtime', 60_000, async () => { + const registered = await positron.runtime.getRegisteredRuntimes(); + const ggsql = registered.filter(runtime => runtime.languageId === 'ggsql'); + return ggsql.length > 0 ? ggsql : undefined; + }); + + assert.strictEqual( + runtimes.length, + 1, + `expected one ggsql runtime, got ${runtimes.map(r => r.runtimePath).join(', ')}`, + ); + // The kernel inside the extension, with the identity that survives an + // update — not something the runner happened to have installed. + assert.strictEqual(runtimes[0].runtimeId, 'ggsql-bundled'); + assert.strictEqual(runtimes[0].runtimeName, 'ggsql'); + assert.ok( + runtimes[0].runtimePath.includes(path.join('bundled', 'bin')), + `unexpected kernel path ${runtimes[0].runtimePath}`, + ); + }); + + test('the console starts the bundled kernel and runs a query', async () => { + // executeCode starts a session when none is running, so this covers the + // whole path: spawning the binary, the supervisor's Jupyter handshake, + // and a result coming back. The kernel holds an in-memory DuckDB + // session, so the query needs no connection string. + const result = await positron.runtime.executeCode('ggsql', 'SELECT 1 AS n', false); + assert.ok(result, 'executeCode returned no result'); + + const sessions = await positron.runtime.getActiveSessions(); + const ggsqlSessions = sessions.filter( + session => session.runtimeMetadata.languageId === 'ggsql', + ); + assert.strictEqual(ggsqlSessions.length, 1, 'expected exactly one ggsql session'); + assert.strictEqual(ggsqlSessions[0].runtimeMetadata.runtimeId, 'ggsql-bundled'); + }); + + test('a query with a visualisation returns a plot', async () => { + // The reason a ggsql console exists, and a second execution on the + // session the previous test started. + const result = await positron.runtime.executeCode( + 'ggsql', + 'SELECT 1 AS x, 2 AS y VISUALISE x AS x, y AS y DRAW point', + false, + ); + assert.ok(result, 'executeCode returned no result'); + }); +}); diff --git a/ggsql-vscode/src/test/integration/index.ts b/ggsql-vscode/src/test/integration/index.ts new file mode 100644 index 00000000..73b1aed2 --- /dev/null +++ b/ggsql-vscode/src/test/integration/index.ts @@ -0,0 +1,42 @@ +/* + * Entry point for the Positron integration suite. + * + * @posit-dev/positron-test-electron launches Positron and requires this module + * inside its extension host, so the suite drives mocha itself rather than going + * through @vscode/test-cli the way the stock VS Code suites do. + */ + +import * as fs from 'fs'; +import * as path from 'path'; +import Mocha from 'mocha'; + +export function run(): Promise { + const mocha = new Mocha({ + ui: 'tdd', + color: true, + // Starting a session launches the kernel binary and completes a Jupyter + // handshake over ZeroMQ, which is far slower than anything the stock + // suites do. + timeout: 120_000, + }); + + for (const file of fs.readdirSync(__dirname)) { + if (file.endsWith('.test.js')) { + mocha.addFile(path.join(__dirname, file)); + } + } + + return new Promise((resolve, reject) => { + try { + mocha.run(failures => { + if (failures > 0) { + reject(new Error(`${failures} integration test(s) failed`)); + } else { + resolve(); + } + }); + } catch (err) { + reject(err); + } + }); +} diff --git a/ggsql-vscode/src/test/kernelDiscovery.test.ts b/ggsql-vscode/src/test/kernelDiscovery.test.ts index 427e1f99..337d9a79 100644 --- a/ggsql-vscode/src/test/kernelDiscovery.test.ts +++ b/ggsql-vscode/src/test/kernelDiscovery.test.ts @@ -3,10 +3,13 @@ import * as fs from 'fs'; import * as os from 'os'; import * as path from 'path'; import * as vscode from 'vscode'; +import type * as positron from '@posit-dev/positron'; import { + GgsqlRuntimeManager, discoverKernelPaths, generateMetadata, isKernelAccessible, + resolveConfiguredPath, resolveKernelStrategy, selectKernelCandidates, type KernelCandidate, @@ -25,28 +28,88 @@ function tempDir(): string { const binaryName = process.platform === 'win32' ? 'ggsql-jupyter.exe' : 'ggsql-jupyter'; +function realExtension(): vscode.Extension { + const extension = vscode.extensions.getExtension(EXTENSION_ID); + assert.ok(extension, `extension ${EXTENSION_ID} not found`); + return extension; +} + +/** Write an executable stand-in for the kernel into `dir`. */ +function writeStubKernel(dir: string, mode = 0o755): string { + fs.mkdirSync(dir, { recursive: true }); + const kernelPath = path.join(dir, binaryName); + fs.writeFileSync(kernelPath, '#!/bin/sh\nexit 0\n'); + fs.chmodSync(kernelPath, mode); + return kernelPath; +} + /** - * Build an extension directory that looks like an installed platform VSIX, - * with a stand-in for the kernel at bundled/bin/. + * Build a directory that looks like an installed platform VSIX: a kernel at + * bundled/bin/, and the icon generateMetadata reads from the extension folder. */ function extensionDirWithBundle(mode = 0o755): { extensionPath: string; kernelPath: string } { const extensionPath = tempDir(); - const binDir = path.join(extensionPath, 'bundled', 'bin'); - fs.mkdirSync(binDir, { recursive: true }); - const kernelPath = path.join(binDir, binaryName); - fs.writeFileSync(kernelPath, '#!/bin/sh\nexit 0\n', { mode }); - fs.chmodSync(kernelPath, mode); - return { extensionPath, kernelPath }; + fs.mkdirSync(path.join(extensionPath, 'resources'), { recursive: true }); + fs.copyFileSync( + path.join(realExtension().extensionPath, 'resources', 'ggsql-icon.svg'), + path.join(extensionPath, 'resources', 'ggsql-icon.svg'), + ); + return { extensionPath, kernelPath: writeStubKernel(path.join(extensionPath, 'bundled', 'bin'), mode) }; } function contextFor(extensionPath: string): vscode.ExtensionContext { return { extensionPath } as vscode.ExtensionContext; } +/** + * True when a native installer has put a kernel on this machine. Those paths are + * hard-coded absolutes that no environment variable can redirect, so a test + * needing "no host kernel anywhere" has to stand aside on such a machine. CI + * never has one, which is where the regression matters. + */ +function systemInstallPresent(): boolean { + return [ + '/usr/local/bin/ggsql-jupyter', + '/usr/bin/ggsql-jupyter', + '/Applications/ggsql.app/Contents/MacOS/ggsql-jupyter', + path.join(process.env.PROGRAMFILES || 'C:\\Program Files', 'ggsql', 'ggsql-jupyter.exe'), + ].some(p => fs.existsSync(p)); +} + +// Environment host discovery reads. Saved and restored around any test that +// redirects it, so no other suite sees a doctored environment. +const HOST_ENV_KEYS = ['HOME', 'USERPROFILE', 'APPDATA', 'LOCALAPPDATA', 'PATH'] as const; +let savedEnv: Partial> = {}; + +function isolateHostEnv(homeDir: string): void { + for (const key of HOST_ENV_KEYS) { + savedEnv[key] = process.env[key]; + } + process.env.HOME = homeDir; + process.env.USERPROFILE = homeDir; + process.env.APPDATA = path.join(homeDir, 'AppData', 'Roaming'); + process.env.LOCALAPPDATA = path.join(homeDir, 'AppData', 'Local'); + // An empty directory as PATH makes the which/where lookup fail, so whatever + // the developer has installed cannot contribute a candidate. + process.env.PATH = tempDir(); +} + +function restoreHostEnv(): void { + for (const [key, value] of Object.entries(savedEnv)) { + if (value === undefined) { + delete process.env[key]; + } else { + process.env[key] = value; + } + } + savedEnv = {}; +} + /** * A WorkspaceConfiguration stub covering the two members - * resolveKernelStrategy() uses. Using a stub rather than writing real settings - * keeps the migration cases independent of the test instance's config state. + * resolveKernelStrategy() uses. Using a stub keeps the precedence cases + * independent of the test instance's settings; the suite below pins the same + * behaviour against the real configuration service. */ function fakeConfig(values: { strategy?: { global?: string; workspace?: string; workspaceFolder?: string }; @@ -79,11 +142,13 @@ suite('kernel strategy', () => { test('the manifest declares bundled as the default', () => { // The rest of the suite assumes this default; it is also what makes the // extension work with no kernel installed. - const extension = vscode.extensions.getExtension(EXTENSION_ID); - const property = extension?.packageJSON.contributes.configuration.properties['ggsql.kernelStrategy']; + const property = realExtension().packageJSON.contributes.configuration.properties['ggsql.kernelStrategy']; assert.ok(property, 'ggsql.kernelStrategy is not contributed'); assert.strictEqual(property.default, 'bundled'); assert.deepStrictEqual(property.enum, ['bundled', 'environment', 'path']); + // A shorter enumDescriptions silently misaligns the settings UI, pairing + // each description with the wrong value. + assert.strictEqual(property.enumDescriptions.length, property.enum.length); }); test('an unset strategy resolves to bundled', () => { @@ -178,6 +243,82 @@ suite('kernel candidate selection', () => { }); }); +suite('kernel strategy from real settings', () => { + // The stubbed inspect() above cannot prove any of this: only the real + // configuration service distinguishes a set value from a default, and only + // discoverKernelPaths proves the settings actually reach the precedence rule. + const config = () => vscode.workspace.getConfiguration('ggsql'); + + async function set(key: string, value: string | undefined): Promise { + await config().update(key, value, vscode.ConfigurationTarget.Global); + } + + teardown(async () => { + await set('kernelStrategy', undefined); + await set('kernelPath', undefined); + }); + + test('an unset strategy resolves to the declared default', () => { + assert.strictEqual(resolveKernelStrategy(config()), 'bundled'); + }); + + test('a kernelPath in real settings migrates to the path strategy', async () => { + await set('kernelPath', '/opt/ggsql/ggsql-jupyter'); + assert.strictEqual(resolveKernelStrategy(config()), 'path'); + }); + + test('an explicitly set strategy wins over a configured path', async () => { + await set('kernelPath', '/opt/ggsql/ggsql-jupyter'); + await set('kernelStrategy', 'environment'); + assert.strictEqual(resolveKernelStrategy(config()), 'environment'); + }); + + test('the path strategy discovers the configured kernel', async () => { + const configured = writeStubKernel(tempDir()); + await set('kernelStrategy', 'path'); + await set('kernelPath', configured); + // A bundled kernel is present and must lose to the setting. + const { extensionPath } = extensionDirWithBundle(); + assert.deepStrictEqual( + discoverKernelPaths(contextFor(extensionPath)), + [{ kernelPath: configured, source: 'Setting' }], + ); + }); + + test('the environment strategy puts the bundled kernel last', async () => { + const { extensionPath, kernelPath } = extensionDirWithBundle(); + await set('kernelStrategy', 'environment'); + const candidates = discoverKernelPaths(contextFor(extensionPath)); + // Whether this machine has host kernels is unknown, but the bundled one + // is the fallback either way, so it comes last. + assert.strictEqual(candidates.at(-1)?.source, 'Bundled'); + assert.strictEqual(candidates.at(-1)?.kernelPath, kernelPath); + }); +}); + +suite('resolving a configured kernel path', () => { + test('an absolute path is used as given', () => { + const configured = path.join(tempDir(), binaryName); + assert.strictEqual(resolveConfiguredPath(configured), configured); + }); + + test('a bare name is looked up on PATH', () => { + const name = process.platform === 'win32' ? 'cmd.exe' : 'sh'; + assert.ok( + path.isAbsolute(resolveConfiguredPath(name)), + `${name} did not resolve to an absolute path`, + ); + }); + + test('a bare name that is not on PATH is kept, then rejected', async () => { + // Kept rather than dropped so that discovery reports the user's setting + // as inaccessible instead of ignoring it without a word. + const name = 'ggsql-jupyter-not-a-real-binary'; + assert.strictEqual(resolveConfiguredPath(name), name); + assert.strictEqual(await isKernelAccessible(name), false); + }); +}); + suite('kernel accessibility', () => { test('a bare binary name is not accessible', async () => { // Anything non-absolute reaching this check means the PATH lookup @@ -186,8 +327,7 @@ suite('kernel accessibility', () => { }); test('an executable file is accessible', async () => { - const { kernelPath } = extensionDirWithBundle(); - assert.strictEqual(await isKernelAccessible(kernelPath), true); + assert.strictEqual(await isKernelAccessible(writeStubKernel(tempDir())), true); }); test('a missing file is not accessible', async () => { @@ -208,18 +348,6 @@ suite('bundled kernel discovery', () => { assert.deepStrictEqual(candidates, [{ kernelPath, source: 'Bundled' }]); }); - test('discovery never returns a path that is not on disk', () => { - // With no bundle, discovery falls through to the host locations. What is - // installed on the test machine is unknown, but every candidate it - // reports has to be a real absolute path. - const candidates = discoverKernelPaths(contextFor(tempDir())); - for (const candidate of candidates) { - assert.notStrictEqual(candidate.source, 'Bundled'); - assert.ok(path.isAbsolute(candidate.kernelPath), `${candidate.kernelPath} is not absolute`); - assert.ok(fs.existsSync(candidate.kernelPath), `${candidate.kernelPath} does not exist`); - } - }); - test('a bundled kernel missing its executable bit is repaired', function () { // Insurance against an unpack that drops the bit: without the repair the // binary would be dropped as inaccessible and no runtime would appear. @@ -233,14 +361,146 @@ suite('bundled kernel discovery', () => { }); }); +suite('host kernel discovery', () => { + let home: string; + + setup(() => { + home = tempDir(); + isolateHostEnv(home); + }); + + teardown(() => { + restoreHostEnv(); + }); + + test('a user Jupyter kernelspec is found when the build has no kernel', function () { + if (systemInstallPresent()) { + this.skip(); + } + const kernel = writeStubKernel(path.join(home, '.local', 'share', 'jupyter', 'kernels', 'ggsql')); + const candidates = discoverKernelPaths(contextFor(tempDir())); + assert.deepStrictEqual(candidates, [{ kernelPath: kernel, source: 'Jupyter' }]); + for (const candidate of candidates) { + assert.ok(path.isAbsolute(candidate.kernelPath), `${candidate.kernelPath} is not absolute`); + assert.ok(fs.existsSync(candidate.kernelPath), `${candidate.kernelPath} does not exist`); + } + }); + + test('one kernel reachable by two paths is reported once', function () { + // The realistic duplicate is a kernelspec symlinked to the installed + // binary. Both the macOS and Linux kernelspec locations are checked on + // every platform, so two of them can name one file. + if (process.platform === 'win32' || systemInstallPresent()) { + this.skip(); + } + const real = writeStubKernel(path.join(home, 'opt')); + for (const dir of [ + path.join(home, 'Library', 'Jupyter', 'kernels', 'ggsql'), + path.join(home, '.local', 'share', 'jupyter', 'kernels', 'ggsql'), + ]) { + fs.mkdirSync(dir, { recursive: true }); + fs.symlinkSync(real, path.join(dir, binaryName)); + } + const candidates = discoverKernelPaths(contextFor(tempDir())); + assert.strictEqual( + candidates.length, + 1, + `expected one candidate, got ${candidates.map(c => c.kernelPath).join(', ')}`, + ); + }); + + test('a bundled kernel outranks an installed one', () => { + const hostKernel = writeStubKernel(path.join(home, '.local', 'share', 'jupyter', 'kernels', 'ggsql')); + const { extensionPath, kernelPath } = extensionDirWithBundle(); + assert.deepStrictEqual( + discoverKernelPaths(contextFor(extensionPath)), + [{ kernelPath, source: 'Bundled' }], + ); + assert.ok(fs.existsSync(hostKernel), 'the host kernel was never there to be outranked'); + }); +}); + +suite('runtime registration', () => { + async function collect( + runtimes: AsyncGenerator, + ): Promise { + const collected: positron.LanguageRuntimeMetadata[] = []; + for await (const runtime of runtimes) { + collected.push(runtime); + } + return collected; + } + + function managerFor(extensionPath: string, kernelSpecDir: string): GgsqlRuntimeManager { + const context = { + extensionPath, + extension: { packageJSON: { version: realExtension().packageJSON.version } }, + } as unknown as vscode.ExtensionContext; + return new GgsqlRuntimeManager(context, { kernelSpecDir }); + } + + test('the bundled kernel is registered as a single runtime', async () => { + const { extensionPath, kernelPath } = extensionDirWithBundle(); + const runtimes = await collect(managerFor(extensionPath, tempDir()).discoverAllRuntimes()); + assert.strictEqual(runtimes.length, 1); + assert.strictEqual(runtimes[0].runtimeId, 'ggsql-bundled'); + assert.strictEqual(runtimes[0].runtimePath, kernelPath); + assert.strictEqual(runtimes[0].runtimeName, 'ggsql'); + }); + + test('discovery advertises the bundled kernel to Jupyter', async () => { + // Quarto and Jupyter resolve ggsql through this spec. It is rewritten on + // every window open because an extension update leaves the previous one + // pointing into a directory that no longer exists. + const { extensionPath, kernelPath } = extensionDirWithBundle(); + const kernelSpecDir = tempDir(); + await collect(managerFor(extensionPath, kernelSpecDir).discoverAllRuntimes()); + const spec = JSON.parse(fs.readFileSync(path.join(kernelSpecDir, 'kernel.json'), 'utf8')); + assert.strictEqual(spec.argv[0], kernelPath); + assert.strictEqual(spec.language, 'ggsql'); + }); + + test('a bundled path that is not an executable file registers nothing', async () => { + // The accessibility filter is what stands between a broken bundle and a + // runtime that fails at session start. A directory where the binary + // should be exists and carries the executable bit, so only the isFile() + // check rejects it — and no kernel spec may be written either. + const extensionPath = tempDir(); + fs.mkdirSync(path.join(extensionPath, 'resources'), { recursive: true }); + fs.copyFileSync( + path.join(realExtension().extensionPath, 'resources', 'ggsql-icon.svg'), + path.join(extensionPath, 'resources', 'ggsql-icon.svg'), + ); + fs.mkdirSync(path.join(extensionPath, 'bundled', 'bin', binaryName), { recursive: true }); + + const kernelSpecDir = tempDir(); + const runtimes = await collect(managerFor(extensionPath, kernelSpecDir).discoverAllRuntimes()); + assert.deepStrictEqual(runtimes, []); + assert.strictEqual(fs.existsSync(path.join(kernelSpecDir, 'kernel.json')), false); + }); + + test('a machine with no kernel at all registers nothing', async function () { + if (systemInstallPresent()) { + this.skip(); + } + // The W6 requirement stated in terms of what Positron receives, rather + // than what the precedence rule returns. + isolateHostEnv(tempDir()); + try { + const runtimes = await collect(managerFor(tempDir(), tempDir()).discoverAllRuntimes()); + assert.deepStrictEqual(runtimes, []); + } finally { + restoreHostEnv(); + } + }); +}); + suite('runtime metadata', () => { // generateMetadata reads resources/ggsql-icon.svg from the extension folder, // so these use the real one with a stand-in version. - function realContext(): vscode.ExtensionContext { - const extension = vscode.extensions.getExtension(EXTENSION_ID); - assert.ok(extension, `extension ${EXTENSION_ID} not found`); + function context(): vscode.ExtensionContext { return { - extensionPath: extension.extensionPath, + extensionPath: realExtension().extensionPath, extension: { packageJSON: { version: '9.9.9' } }, } as unknown as vscode.ExtensionContext; } @@ -249,12 +509,11 @@ suite('runtime metadata', () => { // The bundled kernel lives under the versioned extension directory. An // id derived from that path would change on every update, dropping the // workspace's runtime affinity and its restorable sessions. - const context = realContext(); - const before = generateMetadata(context, { + const before = generateMetadata(context(), { kernelPath: '/ext/ggsql.ggsql-0.5.0-darwin-arm64/bundled/bin/ggsql-jupyter', source: 'Bundled', }); - const after = generateMetadata(context, { + const after = generateMetadata(context(), { kernelPath: '/ext/ggsql.ggsql-0.6.0-darwin-arm64/bundled/bin/ggsql-jupyter', source: 'Bundled', }); @@ -263,7 +522,7 @@ suite('runtime metadata', () => { test('the bundled runtime is named plain ggsql', () => { // It is the default, so there is nothing to distinguish it from. - const metadata = generateMetadata(realContext(), { + const metadata = generateMetadata(context(), { kernelPath: '/ext/ggsql.ggsql-0.5.0/bundled/bin/ggsql-jupyter', source: 'Bundled', }); @@ -271,12 +530,11 @@ suite('runtime metadata', () => { }); test('other runtimes keep a per-path id and a qualified name', () => { - const context = realContext(); - const system = generateMetadata(context, { + const system = generateMetadata(context(), { kernelPath: '/usr/local/bin/ggsql-jupyter', source: 'System', }); - const setting = generateMetadata(context, { + const setting = generateMetadata(context(), { kernelPath: '/opt/ggsql/ggsql-jupyter', source: 'Setting', }); diff --git a/ggsql-vscode/src/test/runIntegration.ts b/ggsql-vscode/src/test/runIntegration.ts new file mode 100644 index 00000000..80fdd8b7 --- /dev/null +++ b/ggsql-vscode/src/test/runIntegration.ts @@ -0,0 +1,50 @@ +/* + * Downloads Positron and runs the integration suite against it. + * + * Invoked by `npm run test:integration`. The stock VS Code suites go through + * @vscode/test-cli instead; only this suite needs a real Positron, because only + * it touches the language runtime API. + */ + +import * as fs from 'fs'; +import * as path from 'path'; +import { runTests } from '@posit-dev/positron-test-electron'; + +async function main(): Promise { + // out-test/test/runIntegration.js -> the extension root + const extensionDevelopmentPath = path.resolve(__dirname, '..', '..'); + const extensionTestsPath = path.resolve(__dirname, 'integration', 'index'); + + const binaryName = process.platform === 'win32' ? 'ggsql-jupyter.exe' : 'ggsql-jupyter'; + const bundledKernel = path.join(extensionDevelopmentPath, 'bundled', 'bin', binaryName); + if (!fs.existsSync(bundledKernel)) { + // Failing here names the missing fixture, rather than letting the suite + // fail later on an absent runtime. + throw new Error( + `no bundled kernel at ${bundledKernel}\n` + + 'Build one first:\n' + + ' cargo build --release --bin ggsql-jupyter\n' + + ` mkdir -p ${path.dirname(bundledKernel)} && cp target/release/${binaryName} ${bundledKernel}`, + ); + } + + const code = await runTests({ + extensionDevelopmentPath, + extensionTestsPath, + // Positron's stable channel is not published for every platform, and the + // daily build is what the extension is developed against. + channel: 'daily', + // The runtime needs positron.positron-supervisor, one of Positron's + // bundled extensions, to start a session at all. With the default + // --disable-extensions there would be no supervisor and every session + // start would fail. + disableExtensions: false, + }); + + process.exit(code); +} + +main().catch(err => { + console.error(err); + process.exit(1); +}); From 93737e129d0f7f608a21317c178ea3f50d987a2d Mon Sep 17 00:00:00 2001 From: Sam Clark Date: Tue, 18 Aug 2026 12:06:54 -0500 Subject: [PATCH 5/8] Stop test caches leaking into the packaged VSIX MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Packaging from a working tree that had run the tests swept 269 stray .d.ts files out of .vscode-test/ and .positron-test/ into the VSIX: both directories are excluded, but the later `!**/*.d.ts` re-included every .d.ts anywhere, and in .vscodeignore the last matching rule wins. Nothing reads a .d.ts at runtime — esbuild bundles the one dependency — so the negation only ever shipped junk, and it goes. The Positron download cache also needed excluding outright. Without it `vsce package` walked all 2.9 GB of it and died in the secret scanner on a directory symlink inside the app bundle. A clean checkout was unaffected, which is why CI never saw it, so the packaging check moves into the job that has just run the tests and therefore has a populated .vscode-test/. That also drops a job rather than adding one, and it now asserts the absence of both caches. Measured on darwin-arm64 with a real kernel: 46.3 MiB binary, 16.21 MiB VSIX across 16 files, against the plan's ~15.6 MiB projection. The kernel-less universal build is 106 KB. Co-Authored-By: Claude Opus 5 (1M context) --- .github/workflows/test-extension.yaml | 45 ++++++++++----------------- ggsql-vscode/.vscodeignore | 2 +- 2 files changed, 17 insertions(+), 30 deletions(-) diff --git a/.github/workflows/test-extension.yaml b/.github/workflows/test-extension.yaml index 71130bac..49d37dd9 100644 --- a/.github/workflows/test-extension.yaml +++ b/.github/workflows/test-extension.yaml @@ -60,41 +60,28 @@ jobs: if: runner.os != 'Linux' run: npm test - # Packaging invariants the release workflow depends on. That workflow only - # runs on a tag, so without this a broken .vscodeignore or manifest would not - # surface until release time. No kernel is staged here: this is the - # kernel-less universal build, which must stay kernel-less. - test-packaging: - runs-on: ubuntu-latest - name: Package (universal VSIX) - defaults: - run: - working-directory: ggsql-vscode - - steps: - - name: Check out repository - uses: actions/checkout@v4 - - - name: Setup Node.js - uses: actions/setup-node@v4 - with: - node-version: "22" - cache: npm - cache-dependency-path: ggsql-vscode/package-lock.json - - - name: Install dependencies - run: npm ci - + # Packaging invariants the release workflow depends on. That workflow only + # runs on a tag, so on its own a broken .vscodeignore would not surface + # until release time. + # + # This runs *after* the tests, deliberately: by now .vscode-test/ holds a + # downloaded VS Code, which is the state that catches an ignore rule + # letting a test cache into the package. A fresh checkout cannot. - name: Package VSIX + if: runner.os == 'Linux' run: npx --yes @vscode/vsce package --out ggsql-universal.vsix - name: Check the VSIX contents + if: runner.os == 'Linux' run: | unzip -l ggsql-universal.vsix - if unzip -l ggsql-universal.vsix | grep -q 'extension/bundled/'; then - echo "::error::the universal VSIX must not carry a kernel" - exit 1 - fi + # No kernel was staged, so this is the universal build. + for unwanted in 'extension/bundled/' '\.vscode-test' '\.positron-test' '\.d\.ts$'; do + if unzip -l ggsql-universal.vsix | grep -q "$unwanted"; then + echo "::error::$unwanted must not be in the VSIX" + exit 1 + fi + done for required in extension/out/extension.js extension/syntaxes/ggsql.tmLanguage.json extension/resources/ggsql-icon.svg; do if ! unzip -l ggsql-universal.vsix | grep -q "$required"; then echo "::error::$required is missing from the VSIX" diff --git a/ggsql-vscode/.vscodeignore b/ggsql-vscode/.vscodeignore index 8dcb9cb3..9e2fbb6f 100644 --- a/ggsql-vscode/.vscodeignore +++ b/ggsql-vscode/.vscodeignore @@ -1,5 +1,6 @@ .vscode/** .vscode-test/** +.positron-test/** .gitignore .yarnrc vsc-extension-quickstart.md @@ -8,7 +9,6 @@ tsconfig.test.json **/.eslintrc.json **/*.map **/*.ts -!**/*.d.ts node_modules/** .editorconfig src/** From b08f30c347ec8b773e5ae69d3dbd08fc61e4327a Mon Sep 17 00:00:00 2001 From: Sam Clark Date: Wed, 19 Aug 2026 14:04:08 -0500 Subject: [PATCH 6/8] Fall back to an installed kernel when the bundled one cannot run A bundled kernel is built for a platform, not for every system it can be installed on. One linked against newer shared libraries than the host provides is exec'd successfully and then rejected by the dynamic linker, so it passes every filesystem check discovery makes and still cannot serve a session. On the Linux builds this is the common case rather than an edge one: the kernels are built on Ubuntu 24.04 and need GLIBC_2.39, while Positron supports back to Ubuntu 20.04 and RHEL 9. Run the bundled kernel before offering it, and put the host locations behind it as a fallback tier. selectKernelCandidates() now returns a KernelSelection carrying that tier as a callback, so the common case -- a bundled kernel that runs -- never pays for the PATH lookup. Only the bundled kernel is probed; a kernel the user installed is taken at its word. A success is cached against the extension version, keeping it to one spawn per update; a failure is not, so a host that gains the missing libraries starts working without waiting for an update. The Jupyter kernel spec is written only for a kernel that passed, because it outlives the window, is what Quarto resolves, and has no fallback. A fallback that succeeds stays silent: the runtime's name in the picker already discloses where it came from. Only the dead end interrupts -- nothing runnable anywhere, whether the bundled kernel failed or the build carries none -- with one non-modal notice per extension version. ggsql-jupyter gains --version, which the probe uses and which had no way to be asked before. Co-Authored-By: Claude Opus 5 (1M context) --- CHANGELOG.md | 11 + ggsql-jupyter/src/main.rs | 3 + ggsql-vscode/CHANGELOG.md | 4 + ggsql-vscode/CLAUDE.md | 11 +- ggsql-vscode/src/extension.ts | 5 + ggsql-vscode/src/manager.ts | 262 ++++++++++++-- ggsql-vscode/src/test/kernelDiscovery.test.ts | 333 ++++++++++++++++-- 7 files changed, 563 insertions(+), 66 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index e4d96cda..15826135 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -47,6 +47,17 @@ a kernel installed on the machine, and a fixed path in `ggsql.kernelPath`; configuring `ggsql.kernelPath` alone continues to mean that path is used. +- The extension now runs the bundled kernel before offering it, and falls back to + a kernel installed on the machine when it does not start. A bundled binary can + be built against newer system libraries than the host provides — every + filesystem check passes and the kernel still dies the moment it is launched — + so the extension runs `ggsql-jupyter --version` once per update and treats a + failure as "not usable here". When nothing on the machine can run, including + builds that carry no kernel at all, the extension says so once and points at + the install instructions instead of offering a runtime that cannot start. + +- `ggsql-jupyter` accepts `--version`. + ### Changed - Dodging now only takes effect where groups actually meet on a position. A layer whose grouping gives every group a position of its own — `colour` mapped diff --git a/ggsql-jupyter/src/main.rs b/ggsql-jupyter/src/main.rs index fccb8906..b9e7f94d 100644 --- a/ggsql-jupyter/src/main.rs +++ b/ggsql-jupyter/src/main.rs @@ -18,6 +18,9 @@ use std::process::Command; #[derive(Parser)] #[command(name = "ggsql-jupyter")] #[command(about = "Jupyter kernel for ggsql", long_about = None)] +// `--version` doubles as a liveness probe: the VS Code extension runs it to +// confirm a kernel binary actually loads on this machine before offering it. +#[command(version)] struct Args { /// Path to the Jupyter connection file #[arg(short = 'f', long = "connection-file")] diff --git a/ggsql-vscode/CHANGELOG.md b/ggsql-vscode/CHANGELOG.md index eaa6ab97..47bab110 100644 --- a/ggsql-vscode/CHANGELOG.md +++ b/ggsql-vscode/CHANGELOG.md @@ -8,6 +8,10 @@ `ggsql.kernelPath`. - Fixed: no ggsql runtime is offered when no kernel can be found, rather than one that fails at session start with `KS-19: Kernel path not found`. +- The bundled kernel is run before it is offered, and a kernel installed on the + machine is used instead when it does not start. If nothing on the machine can + run, the extension points at the install instructions once rather than offering + a runtime that fails at session start. ## 0.3.2 diff --git a/ggsql-vscode/CLAUDE.md b/ggsql-vscode/CLAUDE.md index c999ebe1..ff7323cd 100644 --- a/ggsql-vscode/CLAUDE.md +++ b/ggsql-vscode/CLAUDE.md @@ -116,22 +116,25 @@ The extension ships the kernel: the per-platform VSIXes carry `ggsql-jupyter` at | Strategy | Candidates, in priority order | | --- | --- | -| `bundled` (default) | The bundled kernel alone. A build that carries none falls through to the host locations, so the platform-neutral VSIX behaves as it always did. | +| `bundled` (default) | The bundled kernel, with host locations behind it as a fallback tier reached only when it cannot run. A build that carries no kernel goes straight to the host locations. | | `environment` | Host locations, then the bundled kernel as the fallback. | | `path` | `ggsql.kernelPath` alone — neither the bundled kernel nor a host install stands in for it. An empty path is treated as `bundled`. | Host locations are, in order: Jupyter kernelspec directories (user then system), the native package install locations per platform, then `PATH`. -`selectKernelCandidates()` is the whole precedence rule with no filesystem in it, which is what `src/test/kernelDiscovery.test.ts` exercises; `discoverKernelPaths()` supplies it with what is actually on disk. +`selectKernelCandidates()` is the whole precedence rule with no filesystem in it, which is what `src/test/kernelDiscovery.test.ts` exercises; `discoverKernelPaths()` supplies it with what is actually on disk. It returns a `KernelSelection`: the `candidates` to offer, plus a `fallback()` callback for the tier behind them. The fallback is a callback rather than a list because reaching the host locations shells out to `which`/`where`, and the common case — a bundled kernel that runs — must not pay for it. -Four things here are load bearing: +Five things here are load bearing: +- **A kernel is run before it is offered.** Filesystem checks cannot tell whether a binary starts. The bundled kernel is built for the platform but not for every system it can be installed on: one linked against newer shared libraries than the host provides is exec'd successfully and then killed by the dynamic linker, which no `stat` or `access` call can see. `probeKernel()` runs `ggsql-jupyter --version` and requires exit 0; only the bundled kernel is probed, since a kernel the user installed is their own business. A success is cached in `globalState` against the extension version, so it costs one spawn per update rather than one per window; a failure is not cached, because it is cheap to repeat and a host that gains the missing libraries should start working without waiting for an update. - **Every candidate is an absolute path.** A candidate that is only a binary name satisfies each existence check further down and so registers a runtime that fails at session start with `KS-19: Kernel path not found`. `findOnPath()` returns `undefined` rather than the bare name, and `isKernelAccessible()` rejects any non-absolute path, so no kernel anywhere means **zero** runtimes rather than an unusable one. The single exception is a `ggsql.kernelPath` that resolves to nothing: it is passed through so discovery can report it as inaccessible in the log instead of ignoring the setting silently. - **The bundled kernel's `runtimeId` is fixed, not derived from its path.** Every other source hashes `kernelPath` to get one id per installed kernel, but the bundled path contains the versioned extension directory, so hashing it would mint a new runtime on every extension update and lose the workspace's runtime affinity and its restorable sessions. - **The bundled runtime is named plain `ggsql`.** The `ggsql ()` suffix is only worth showing for a kernel the user went out of their way to select. - **`ggsql.kernelPath` implies `path`.** Users configured that setting before a strategy existed, so a non-empty path with no explicitly set `kernelStrategy` still resolves to `path`. `resolveKernelStrategy()` reads the value through `inspect()` for that reason: `get()` cannot tell a set value from the default. -Discovery also writes the user-level Jupyter kernelspec for the bundled and system kernels, so Quarto and Jupyter can find ggsql without a session ever being started, and so the spec stops pointing into an extension directory an update has removed. +Discovery also writes the user-level Jupyter kernelspec for the bundled and system kernels, so Quarto and Jupyter can find ggsql without a session ever being started, and so the spec stops pointing into an extension directory an update has removed. Only a kernel that has passed the probe is written there: the spec outlives the window and is what Quarto resolves, and it has no fallback of its own. + +A fallback that succeeds is deliberately silent — the runtime's name in the picker already says where it came from, and the log records the handover. The one case that interrupts the user is the dead end: nothing runnable anywhere, whether because the bundled kernel failed its probe or because the build carries none (the platform-neutral VSIX). `reportNoUsableKernel()` then shows a non-modal warning once per extension version, offering the install docs and the log. It is skipped under the `path` strategy, where the user named a binary and the log already reports it. ## Settings diff --git a/ggsql-vscode/src/extension.ts b/ggsql-vscode/src/extension.ts index 435f26a0..b9d1c6be 100644 --- a/ggsql-vscode/src/extension.ts +++ b/ggsql-vscode/src/extension.ts @@ -23,6 +23,11 @@ export function log(message: string): void { outputChannel.appendLine(`[${new Date().toISOString()}] ${message}`); } +/** Reveal the ggsql output channel, for notifications that offer it. */ +export function showLog(): void { + outputChannel.show(); +} + /** * Activates the extension. * diff --git a/ggsql-vscode/src/manager.ts b/ggsql-vscode/src/manager.ts index b21cd7df..360abf09 100644 --- a/ggsql-vscode/src/manager.ts +++ b/ggsql-vscode/src/manager.ts @@ -12,7 +12,7 @@ import * as cp from 'child_process'; import * as crypto from 'crypto'; import type * as positron from '@posit-dev/positron'; import type { JupyterKernelSpec, PositronSupervisorApi } from './types'; -import { log } from './extension'; +import { log, showLog } from './extension'; /** Where a kernel candidate was discovered */ type KernelSource = 'Bundled' | 'Setting' | 'Jupyter' | 'System' | 'Path'; @@ -210,6 +210,25 @@ export function resolveKernelStrategy(config: vscode.WorkspaceConfiguration): Ke return 'bundled'; } +/** + * The kernels a window should consider, in priority order, plus the tier to + * fall back to when none of them can run. + */ +export interface KernelSelection { + /** The strategy these candidates were chosen under. */ + strategy: KernelStrategy; + /** Candidates to offer, best first. */ + candidates: KernelCandidate[]; + /** + * Host kernels to consult when nothing in `candidates` turns out to be + * runnable. A callback, not a list, so that the common case — a bundled + * kernel that runs — never pays for the PATH lookup. + */ + fallback: () => KernelCandidate[]; +} + +const NO_FALLBACK = (): KernelCandidate[] => []; + /** * Apply a strategy to the places a kernel can come from. * @@ -221,25 +240,42 @@ export function selectKernelCandidates( bundledPath: string | undefined, configuredPath: string | undefined, hostKernels: () => KernelCandidate[], -): KernelCandidate[] { +): KernelSelection { const bundled: KernelCandidate[] = bundledPath ? [{ kernelPath: bundledPath, source: 'Bundled' }] : []; + let effective = strategy; if (strategy === 'path') { if (configuredPath) { - return [{ kernelPath: configuredPath, source: 'Setting' }]; + // The user named a binary. Nothing may quietly stand in for it, so + // there is no fallback tier here. + return { + strategy, + candidates: [{ kernelPath: configuredPath, source: 'Setting' }], + fallback: NO_FALLBACK, + }; } // Nothing to point at. Treat it as the default rather than registering // no runtime at all. log('ggsql.kernelStrategy is "path" but ggsql.kernelPath is empty; using the bundled kernel'); + effective = 'bundled'; } else if (strategy === 'environment') { - return [...hostKernels(), ...bundled]; + // Host kernels are already ahead of the bundled one, so a failure among + // them falls through to it within the same tier. + return { strategy, candidates: [...hostKernels(), ...bundled], fallback: NO_FALLBACK }; } + if (bundled.length > 0) { + // The bundled kernel is built for this platform but not for every + // system it can be installed on: it can be too new for the host's + // shared libraries, which only shows up when it is run. Host kernels + // stand behind it for exactly that case. + return { strategy: effective, candidates: bundled, fallback: hostKernels }; + } // A build that carries no kernel still looks for a host install, or it // would offer nothing at all. - return bundled.length > 0 ? bundled : hostKernels(); + return { strategy: effective, candidates: hostKernels(), fallback: NO_FALLBACK }; } /** @@ -270,19 +306,25 @@ function dedupeCandidates(candidates: KernelCandidate[]): KernelCandidate[] { * Discover the ggsql-jupyter kernels this window should offer, in priority * order. */ -export function discoverKernelPaths(context: vscode.ExtensionContext): KernelCandidate[] { +export function discoverKernelPaths(context: vscode.ExtensionContext): KernelSelection { const config = vscode.workspace.getConfiguration('ggsql'); const strategy = resolveKernelStrategy(config); log(`Kernel strategy: ${strategy}`); const configuredPath = config.get('kernelPath', '').trim(); - return dedupeCandidates(selectKernelCandidates( + const selection = selectKernelCandidates( strategy, bundledKernelPath(context), configuredPath === '' ? undefined : resolveConfiguredPath(configuredPath), discoverHostKernels, - )); + ); + + return { + strategy: selection.strategy, + candidates: dedupeCandidates(selection.candidates), + fallback: () => dedupeCandidates(selection.fallback()), + }; } /** @@ -308,6 +350,134 @@ export async function isKernelAccessible(kernelPath: string): Promise { } } +/** How long the probe waits for the kernel to report its version. */ +const KERNEL_PROBE_TIMEOUT_MS = 15000; + +/** Where the last successful probe is remembered, to keep it to one per update. */ +const PROBE_CACHE_KEY = 'ggsql.bundledKernelProbe'; + +/** Where the dead-end notice records the version it has already reported. */ +const NO_KERNEL_NOTICE_KEY = 'ggsql.noUsableKernelNotice'; + +/** Install instructions offered when no kernel on this machine can run. */ +const INSTALL_DOCS_URL = 'https://ggsql.org/get_started/installation.html'; + +interface ProbeCacheEntry { + extensionVersion: string; + kernelPath: string; +} + +/** Runs a kernel binary and reports whether it started. */ +export type KernelProbe = (kernelPath: string) => Promise; + +/** + * Run the kernel and see whether it starts. + * + * An accessibility check cannot answer this. A binary built against newer + * shared libraries than the host provides passes every filesystem test and + * still fails: the kernel is exec'd successfully and then the dynamic linker + * rejects it, so the process exits non-zero before it can serve a session. + * `--version` is the cheapest thing that exercises that whole path. + */ +export function probeKernel(kernelPath: string): Promise { + return new Promise(resolve => { + cp.execFile( + kernelPath, + ['--version'], + { timeout: KERNEL_PROBE_TIMEOUT_MS, windowsHide: true }, + err => { + if (err) { + log(`Kernel probe failed for ${kernelPath}: ${err.message}`); + } + resolve(!err); + }, + ); + }); +} + +/** + * Probe the bundled kernel, remembering a success across windows. + * + * Only a success is cached, and only for the extension version that produced + * it: a failure is cheap to repeat (the linker gives up immediately) and + * re-running it means a host that gains the libraries the kernel needs starts + * working without waiting for an extension update. + */ +async function probeBundledKernel( + context: vscode.ExtensionContext, + kernelPath: string, + probe: KernelProbe, +): Promise { + const extensionVersion = context.extension.packageJSON.version as string; + const cached = context.globalState.get(PROBE_CACHE_KEY); + if (cached?.extensionVersion === extensionVersion && cached.kernelPath === kernelPath) { + return true; + } + + const ok = await probe(kernelPath); + if (ok) { + await context.globalState.update(PROBE_CACHE_KEY, { extensionVersion, kernelPath }); + } + return ok; +} + +/** + * Decide whether a candidate can actually serve a session. + * + * The bundled kernel is additionally run, because it is the one the extension + * chose rather than the user, and it is the one that can be wrong about the + * system it landed on. A kernel the user installed is taken at its word. + */ +async function canRunKernel( + context: vscode.ExtensionContext, + candidate: KernelCandidate, + probe: KernelProbe, +): Promise { + if (!await isKernelAccessible(candidate.kernelPath)) { + return false; + } + if (candidate.source !== 'Bundled') { + return true; + } + return probeBundledKernel(context, candidate.kernelPath, probe); +} + +/** + * Tell the user that nothing on this machine can run ggsql queries. + * + * Only for the dead end: a fallback that succeeds is reported by the runtime's + * name in the picker and by the log, and needs no interruption. Shown once per + * extension version, and never awaited, so discovery does not sit waiting for + * the notification to be dismissed. + */ +function reportNoUsableKernel( + context: vscode.ExtensionContext, + bundledRejected: boolean, +): void { + const version = context.extension.packageJSON.version as string; + if (context.globalState.get(NO_KERNEL_NOTICE_KEY) === version) { + return; + } + void context.globalState.update(NO_KERNEL_NOTICE_KEY, version); + + const reason = bundledRejected + ? 'The ggsql kernel bundled with this extension cannot run on this system.' + : 'This build of the ggsql extension does not include a kernel.'; + log(`${reason} No kernel installed on this machine could be used instead.`); + + const install = 'Install ggsql'; + const showOutput = 'Show Log'; + void vscode.window + .showWarningMessage(`${reason} Install ggsql to run queries.`, install, showOutput) + .then(choice => { + if (choice === install) { + void vscode.env.openExternal(vscode.Uri.parse(INSTALL_DOCS_URL)); + } else if (choice === showOutput) { + showLog(); + } + }); +} + /** * Stable runtime identifier for a candidate. * @@ -512,6 +682,15 @@ export interface RuntimeManagerOptions { * kernelspec — the one Quarto and Jupyter resolve — at a test fixture. */ kernelSpecDir?: string; + + /** + * Liveness probe for the bundled kernel. Defaults to running it. + * + * Tests override it because a stand-in kernel cannot be a real executable + * on every platform: a shell script named ggsql-jupyter.exe is not + * something Windows can spawn. + */ + probe?: KernelProbe; } /** @@ -533,10 +712,12 @@ export class GgsqlRuntimeManager implements positron.LanguageRuntimeManager { private _context: vscode.ExtensionContext; private _kernelSpecDir: string; + private _probe: KernelProbe; constructor(context: vscode.ExtensionContext, options: RuntimeManagerOptions = {}) { this._context = context; this._kernelSpecDir = options.kernelSpecDir ?? getUserJupyterKernelDir(); + this._probe = options.probe ?? probeKernel; } /** @@ -547,33 +728,62 @@ export class GgsqlRuntimeManager implements positron.LanguageRuntimeManager { discoverAllRuntimes(): AsyncGenerator { const context = this._context; const kernelSpecDir = this._kernelSpecDir; + const probe = this._probe; const generator = async function* discoverGgsqlRuntimes() { log('Discovering ggsql runtimes...'); - const candidates = discoverKernelPaths(context); - log(`Found ${candidates.length} kernel candidate(s)`); - - for (const candidate of candidates) { - const accessible = await isKernelAccessible(candidate.kernelPath); - if (accessible) { - // Write the kernel spec to the user kernelspec dir - // immediately so that Quarto/Jupyter can discover ggsql - // even if no session is ever started. The bundled kernel - // additionally needs this on every extension update, or the - // spec keeps pointing into the removed extension directory. - if (candidate.source === 'System' || candidate.source === 'Bundled') { - writeKernelJson(kernelSpecDir, candidate.kernelPath); + const selection = discoverKernelPaths(context); + log(`Found ${selection.candidates.length} kernel candidate(s)`); + + let registered = 0; + let bundledRejected = false; + + async function* offer(candidates: KernelCandidate[]) { + for (const candidate of candidates) { + if (await canRunKernel(context, candidate, probe)) { + // Write the kernel spec to the user kernelspec dir + // immediately so that Quarto/Jupyter can discover ggsql + // even if no session is ever started. The bundled kernel + // additionally needs this on every extension update, or + // the spec keeps pointing into the removed extension + // directory. Only a kernel that has proven it runs is + // advertised this way: the spec outlives the window, and + // Quarto has no discovery of its own to fall back on. + if (candidate.source === 'System' || candidate.source === 'Bundled') { + writeKernelJson(kernelSpecDir, candidate.kernelPath); + } + + const metadata = generateMetadata(context, candidate); + log(`Yielding runtime: ${metadata.runtimeName} (${metadata.runtimeId}) at ${candidate.kernelPath}`); + registered++; + yield metadata; + } else { + if (candidate.source === 'Bundled') { + bundledRejected = true; + } + log(`Skipping unusable kernel (${candidate.source}): ${candidate.kernelPath}`); } + } + } + + yield* offer(selection.candidates); - const metadata = generateMetadata(context, candidate); - log(`Yielding runtime: ${metadata.runtimeName} (${metadata.runtimeId}) at ${candidate.kernelPath}`); - yield metadata; - } else { - log(`Skipping inaccessible kernel (${candidate.source}): ${candidate.kernelPath}`); + if (registered === 0) { + const fallback = selection.fallback(); + if (fallback.length > 0) { + log(`No usable kernel among the primary candidates; trying ${fallback.length} installed kernel(s)`); + yield* offer(fallback); } } + if (registered === 0 && selection.strategy !== 'path') { + // Under the path strategy the user named a binary that did not + // work out, which the log already reports; there is nothing to + // install that would change it. + reportNoUsableKernel(context, bundledRejected); + } + log('Runtime discovery complete'); }; diff --git a/ggsql-vscode/src/test/kernelDiscovery.test.ts b/ggsql-vscode/src/test/kernelDiscovery.test.ts index 337d9a79..1608cef0 100644 --- a/ggsql-vscode/src/test/kernelDiscovery.test.ts +++ b/ggsql-vscode/src/test/kernelDiscovery.test.ts @@ -9,10 +9,12 @@ import { discoverKernelPaths, generateMetadata, isKernelAccessible, + probeKernel, resolveConfiguredPath, resolveKernelStrategy, selectKernelCandidates, type KernelCandidate, + type KernelProbe, } from '../manager'; const EXTENSION_ID = 'ggsql.ggsql'; @@ -198,48 +200,69 @@ suite('kernel strategy', () => { suite('kernel candidate selection', () => { const bundled = '/ext/ggsql.ggsql-0.5.0-darwin-arm64/bundled/bin/ggsql-jupyter'; - test('bundled uses only the bundled kernel', () => { - const candidates = selectKernelCandidates('bundled', bundled, undefined, hostKernels); - assert.deepStrictEqual(candidates, [{ kernelPath: bundled, source: 'Bundled' }]); + test('bundled offers only the bundled kernel, with host kernels behind it', () => { + const selection = selectKernelCandidates('bundled', bundled, undefined, hostKernels); + assert.deepStrictEqual(selection.candidates, [{ kernelPath: bundled, source: 'Bundled' }]); + // Reachable only when the bundled kernel turns out not to run. + assert.deepStrictEqual(selection.fallback(), HOST); + }); + + test('the host lookup is not performed while choosing the bundled kernel', () => { + // The PATH lookup shells out to which/where. The default strategy is the + // common case and must not pay for it. + let calls = 0; + const counted = () => { + calls++; + return HOST; + }; + selectKernelCandidates('bundled', bundled, undefined, counted); + assert.strictEqual(calls, 0); }); test('bundled falls back to host kernels when the build carries none', () => { - // The platform-neutral VSIX ships no kernel and must keep working the - // way it does today. - const candidates = selectKernelCandidates('bundled', undefined, undefined, hostKernels); - assert.deepStrictEqual(candidates, HOST); + // The platform-neutral VSIX ships no kernel and must keep working + // through a host install. + const selection = selectKernelCandidates('bundled', undefined, undefined, hostKernels); + assert.deepStrictEqual(selection.candidates, HOST); + assert.deepStrictEqual(selection.fallback(), []); }); test('no bundle and no host kernel yields no candidates', () => { - // Regression test for the phantom runtime: discovery used to append the - // bare binary name unconditionally, so Positron registered a ggsql - // runtime that failed at session start with KS-19. - const candidates = selectKernelCandidates('bundled', undefined, undefined, noHostKernels); - assert.deepStrictEqual(candidates, []); + // Regression test for the phantom runtime: a ggsql runtime registered + // against a kernel that is not there fails at session start with KS-19. + const selection = selectKernelCandidates('bundled', undefined, undefined, noHostKernels); + assert.deepStrictEqual(selection.candidates, []); + assert.deepStrictEqual(selection.fallback(), []); }); test('environment puts host kernels ahead of the bundled one', () => { - const candidates = selectKernelCandidates('environment', bundled, undefined, hostKernels); - assert.deepStrictEqual(candidates, [...HOST, { kernelPath: bundled, source: 'Bundled' }]); + const selection = selectKernelCandidates('environment', bundled, undefined, hostKernels); + // One tier: the bundled kernel already stands behind the host ones. + assert.deepStrictEqual(selection.candidates, [...HOST, { kernelPath: bundled, source: 'Bundled' }]); + assert.deepStrictEqual(selection.fallback(), []); }); test('environment falls back to the bundled kernel', () => { - const candidates = selectKernelCandidates('environment', bundled, undefined, noHostKernels); - assert.deepStrictEqual(candidates, [{ kernelPath: bundled, source: 'Bundled' }]); + const selection = selectKernelCandidates('environment', bundled, undefined, noHostKernels); + assert.deepStrictEqual(selection.candidates, [{ kernelPath: bundled, source: 'Bundled' }]); }); test('path uses the configured kernel alone', () => { // Neither the bundled kernel nor a host install may quietly stand in for - // the one the user named. - const candidates = selectKernelCandidates('path', bundled, '/opt/ggsql/ggsql-jupyter', hostKernels); - assert.deepStrictEqual(candidates, [ + // the one the user named, so there is no fallback tier either. + const selection = selectKernelCandidates('path', bundled, '/opt/ggsql/ggsql-jupyter', hostKernels); + assert.deepStrictEqual(selection.candidates, [ { kernelPath: '/opt/ggsql/ggsql-jupyter', source: 'Setting' }, ]); + assert.deepStrictEqual(selection.fallback(), []); }); test('path with no configured kernel behaves as bundled', () => { - const candidates = selectKernelCandidates('path', bundled, undefined, hostKernels); - assert.deepStrictEqual(candidates, [{ kernelPath: bundled, source: 'Bundled' }]); + const selection = selectKernelCandidates('path', bundled, undefined, hostKernels); + assert.deepStrictEqual(selection.candidates, [{ kernelPath: bundled, source: 'Bundled' }]); + // Including the dead-end notice, which an empty setting must not suppress. + assert.strictEqual(selection.strategy, 'bundled'); + assert.deepStrictEqual(selection.fallback(), HOST); }); }); @@ -280,7 +303,7 @@ suite('kernel strategy from real settings', () => { // A bundled kernel is present and must lose to the setting. const { extensionPath } = extensionDirWithBundle(); assert.deepStrictEqual( - discoverKernelPaths(contextFor(extensionPath)), + discoverKernelPaths(contextFor(extensionPath)).candidates, [{ kernelPath: configured, source: 'Setting' }], ); }); @@ -288,7 +311,7 @@ suite('kernel strategy from real settings', () => { test('the environment strategy puts the bundled kernel last', async () => { const { extensionPath, kernelPath } = extensionDirWithBundle(); await set('kernelStrategy', 'environment'); - const candidates = discoverKernelPaths(contextFor(extensionPath)); + const candidates = discoverKernelPaths(contextFor(extensionPath)).candidates; // Whether this machine has host kernels is unknown, but the bundled one // is the fallback either way, so it comes last. assert.strictEqual(candidates.at(-1)?.source, 'Bundled'); @@ -344,7 +367,7 @@ suite('kernel accessibility', () => { suite('bundled kernel discovery', () => { test('the bundled kernel is the only candidate under the default strategy', () => { const { extensionPath, kernelPath } = extensionDirWithBundle(); - const candidates = discoverKernelPaths(contextFor(extensionPath)); + const candidates = discoverKernelPaths(contextFor(extensionPath)).candidates; assert.deepStrictEqual(candidates, [{ kernelPath, source: 'Bundled' }]); }); @@ -355,7 +378,7 @@ suite('bundled kernel discovery', () => { this.skip(); } const { extensionPath, kernelPath } = extensionDirWithBundle(0o644); - const candidates = discoverKernelPaths(contextFor(extensionPath)); + const candidates = discoverKernelPaths(contextFor(extensionPath)).candidates; assert.deepStrictEqual(candidates, [{ kernelPath, source: 'Bundled' }]); assert.ok(fs.statSync(kernelPath).mode & 0o111, 'the executable bit was not restored'); }); @@ -378,7 +401,7 @@ suite('host kernel discovery', () => { this.skip(); } const kernel = writeStubKernel(path.join(home, '.local', 'share', 'jupyter', 'kernels', 'ggsql')); - const candidates = discoverKernelPaths(contextFor(tempDir())); + const candidates = discoverKernelPaths(contextFor(tempDir())).candidates; assert.deepStrictEqual(candidates, [{ kernelPath: kernel, source: 'Jupyter' }]); for (const candidate of candidates) { assert.ok(path.isAbsolute(candidate.kernelPath), `${candidate.kernelPath} is not absolute`); @@ -401,7 +424,7 @@ suite('host kernel discovery', () => { fs.mkdirSync(dir, { recursive: true }); fs.symlinkSync(real, path.join(dir, binaryName)); } - const candidates = discoverKernelPaths(contextFor(tempDir())); + const candidates = discoverKernelPaths(contextFor(tempDir())).candidates; assert.strictEqual( candidates.length, 1, @@ -412,11 +435,14 @@ suite('host kernel discovery', () => { test('a bundled kernel outranks an installed one', () => { const hostKernel = writeStubKernel(path.join(home, '.local', 'share', 'jupyter', 'kernels', 'ggsql')); const { extensionPath, kernelPath } = extensionDirWithBundle(); + const selection = discoverKernelPaths(contextFor(extensionPath)); + assert.deepStrictEqual(selection.candidates, [{ kernelPath, source: 'Bundled' }]); + // The installed one is not gone, just behind: it is what the fallback + // tier reaches for when the bundled kernel cannot run. assert.deepStrictEqual( - discoverKernelPaths(contextFor(extensionPath)), - [{ kernelPath, source: 'Bundled' }], + selection.fallback(), + [{ kernelPath: hostKernel, source: 'Jupyter' }], ); - assert.ok(fs.existsSync(hostKernel), 'the host kernel was never there to be outranked'); }); }); @@ -431,17 +457,71 @@ suite('runtime registration', () => { return collected; } - function managerFor(extensionPath: string, kernelSpecDir: string): GgsqlRuntimeManager { + /** An in-memory stand-in for context.globalState, which the probe cache uses. */ + function memoryState(): vscode.Memento { + const store = new Map(); + return { + keys: () => [...store.keys()], + get: (key: string, defaultValue?: unknown) => + store.has(key) ? store.get(key) : defaultValue, + update: async (key: string, value: unknown) => { + store.set(key, value); + }, + } as unknown as vscode.Memento; + } + + /** + * A manager over a stand-in extension directory. + * + * The probe defaults to passing: a stand-in kernel cannot be a real + * executable on every platform, so running one for real is left to the + * `kernel probe` suite and these tests inject the verdict instead. + */ + function managerFor( + extensionPath: string, + kernelSpecDir: string, + options: { probe?: KernelProbe; globalState?: vscode.Memento } = {}, + ): { manager: GgsqlRuntimeManager; globalState: vscode.Memento } { + const globalState = options.globalState ?? memoryState(); const context = { extensionPath, + globalState, extension: { packageJSON: { version: realExtension().packageJSON.version } }, } as unknown as vscode.ExtensionContext; - return new GgsqlRuntimeManager(context, { kernelSpecDir }); + const manager = new GgsqlRuntimeManager(context, { + kernelSpecDir, + probe: options.probe ?? (async () => true), + }); + return { manager, globalState }; } + /** The key reportNoUsableKernel stamps once it has warned for this version. */ + const NOTICE_KEY = 'ggsql.noUsableKernelNotice'; + + // The dead-end notice is fire-and-forget, so it is captured rather than + // awaited. Stubbing it also keeps the suite from raising real notifications + // in the test window. + let warnings: string[] = []; + let realShowWarningMessage: typeof vscode.window.showWarningMessage; + + setup(() => { + warnings = []; + realShowWarningMessage = vscode.window.showWarningMessage; + (vscode.window as unknown as Record).showWarningMessage = + (message: string) => { + warnings.push(message); + return Promise.resolve(undefined); + }; + }); + + teardown(() => { + (vscode.window as unknown as Record).showWarningMessage = + realShowWarningMessage; + }); + test('the bundled kernel is registered as a single runtime', async () => { const { extensionPath, kernelPath } = extensionDirWithBundle(); - const runtimes = await collect(managerFor(extensionPath, tempDir()).discoverAllRuntimes()); + const runtimes = await collect(managerFor(extensionPath, tempDir()).manager.discoverAllRuntimes()); assert.strictEqual(runtimes.length, 1); assert.strictEqual(runtimes[0].runtimeId, 'ggsql-bundled'); assert.strictEqual(runtimes[0].runtimePath, kernelPath); @@ -454,7 +534,7 @@ suite('runtime registration', () => { // pointing into a directory that no longer exists. const { extensionPath, kernelPath } = extensionDirWithBundle(); const kernelSpecDir = tempDir(); - await collect(managerFor(extensionPath, kernelSpecDir).discoverAllRuntimes()); + await collect(managerFor(extensionPath, kernelSpecDir).manager.discoverAllRuntimes()); const spec = JSON.parse(fs.readFileSync(path.join(kernelSpecDir, 'kernel.json'), 'utf8')); assert.strictEqual(spec.argv[0], kernelPath); assert.strictEqual(spec.language, 'ggsql'); @@ -474,11 +554,153 @@ suite('runtime registration', () => { fs.mkdirSync(path.join(extensionPath, 'bundled', 'bin', binaryName), { recursive: true }); const kernelSpecDir = tempDir(); - const runtimes = await collect(managerFor(extensionPath, kernelSpecDir).discoverAllRuntimes()); + const runtimes = await collect(managerFor(extensionPath, kernelSpecDir).manager.discoverAllRuntimes()); assert.deepStrictEqual(runtimes, []); assert.strictEqual(fs.existsSync(path.join(kernelSpecDir, 'kernel.json')), false); }); + test('a bundled kernel that cannot run hands over to an installed one', async function () { + // The bundled kernel is built for the platform, not for every system on + // it: one built against newer shared libraries than the host provides + // execs and then dies under the dynamic linker. Nothing on the + // filesystem shows that, so the host install has to be reachable. + if (systemInstallPresent()) { + this.skip(); + } + const home = tempDir(); + isolateHostEnv(home); + try { + const hostKernel = writeStubKernel( + path.join(home, '.local', 'share', 'jupyter', 'kernels', 'ggsql'), + ); + const { extensionPath, kernelPath } = extensionDirWithBundle(); + const { manager } = managerFor(extensionPath, tempDir(), { + probe: async candidate => candidate !== kernelPath, + }); + + const runtimes = await collect(manager.discoverAllRuntimes()); + assert.strictEqual(runtimes.length, 1); + assert.strictEqual(runtimes[0].runtimePath, hostKernel); + // Named for where it came from, which is how the handover is + // disclosed without interrupting the user. + assert.strictEqual(runtimes[0].runtimeName, 'ggsql (Jupyter)'); + // A fallback that works is not worth interrupting anyone over. + assert.deepStrictEqual(warnings, []); + } finally { + restoreHostEnv(); + } + }); + + test('a bundled kernel that cannot run is not advertised to Jupyter', async function () { + // The kernel spec outlives the window and is what Quarto resolves, so + // pointing it at a binary that does not run would break tools that + // never see this extension's fallback. + if (systemInstallPresent()) { + this.skip(); + } + isolateHostEnv(tempDir()); + try { + const { extensionPath } = extensionDirWithBundle(); + const kernelSpecDir = tempDir(); + const { manager } = managerFor(extensionPath, kernelSpecDir, { + probe: async () => false, + }); + + const runtimes = await collect(manager.discoverAllRuntimes()); + assert.deepStrictEqual(runtimes, []); + assert.strictEqual(fs.existsSync(path.join(kernelSpecDir, 'kernel.json')), false); + } finally { + restoreHostEnv(); + } + }); + + test('a bundled kernel that cannot run and no installed one warns once', async function () { + if (systemInstallPresent()) { + this.skip(); + } + isolateHostEnv(tempDir()); + try { + const { extensionPath } = extensionDirWithBundle(); + const globalState = memoryState(); + + const first = managerFor(extensionPath, tempDir(), { + probe: async () => false, + globalState, + }); + assert.deepStrictEqual(await collect(first.manager.discoverAllRuntimes()), []); + assert.strictEqual(warnings.length, 1); + assert.match(warnings[0], /cannot run on this system/); + assert.strictEqual(globalState.get(NOTICE_KEY), realExtension().packageJSON.version); + + // Discovery runs on every window open; the notice must not repeat. + const second = managerFor(extensionPath, tempDir(), { + probe: async () => false, + globalState, + }); + assert.deepStrictEqual(await collect(second.manager.discoverAllRuntimes()), []); + assert.strictEqual(warnings.length, 1, 'the dead-end notice was shown twice'); + } finally { + restoreHostEnv(); + } + }); + + test('a bundled kernel that runs is not re-probed on the next window', async () => { + const { extensionPath } = extensionDirWithBundle(); + const globalState = memoryState(); + let probes = 0; + const probe: KernelProbe = async () => { + probes++; + return true; + }; + + const first = managerFor(extensionPath, tempDir(), { probe, globalState }); + assert.strictEqual((await collect(first.manager.discoverAllRuntimes())).length, 1); + assert.strictEqual(probes, 1); + + const second = managerFor(extensionPath, tempDir(), { probe, globalState }); + assert.strictEqual((await collect(second.manager.discoverAllRuntimes())).length, 1); + assert.strictEqual(probes, 1, 'the bundled kernel was probed again'); + }); + + test('a bundled kernel that runs never reaches for an installed one', async () => { + // The fallback costs a PATH lookup that shells out. The default + // strategy is the common case and must not pay for it. + const home = tempDir(); + isolateHostEnv(home); + try { + writeStubKernel(path.join(home, '.local', 'share', 'jupyter', 'kernels', 'ggsql')); + const { extensionPath, kernelPath } = extensionDirWithBundle(); + const { manager } = managerFor(extensionPath, tempDir()); + + const runtimes = await collect(manager.discoverAllRuntimes()); + assert.strictEqual(runtimes.length, 1); + assert.strictEqual(runtimes[0].runtimePath, kernelPath); + assert.strictEqual(runtimes[0].runtimeName, 'ggsql'); + } finally { + restoreHostEnv(); + } + }); + + test('a build with no kernel and nothing installed warns too', async function () { + // The platform-neutral VSIX carries no kernel at all. That dead end + // is the same one, and gets the same notice. + if (systemInstallPresent()) { + this.skip(); + } + isolateHostEnv(tempDir()); + try { + const globalState = memoryState(); + const { manager } = managerFor(tempDir(), tempDir(), { globalState }); + assert.deepStrictEqual(await collect(manager.discoverAllRuntimes()), []); + assert.strictEqual(warnings.length, 1); + // Worded for a build that never carried a kernel, not a broken one. + assert.match(warnings[0], /does not include a kernel/); + assert.strictEqual(globalState.get(NOTICE_KEY), realExtension().packageJSON.version); + } finally { + restoreHostEnv(); + } + }); + test('a machine with no kernel at all registers nothing', async function () { if (systemInstallPresent()) { this.skip(); @@ -487,7 +709,7 @@ suite('runtime registration', () => { // than what the precedence rule returns. isolateHostEnv(tempDir()); try { - const runtimes = await collect(managerFor(tempDir(), tempDir()).discoverAllRuntimes()); + const runtimes = await collect(managerFor(tempDir(), tempDir()).manager.discoverAllRuntimes()); assert.deepStrictEqual(runtimes, []); } finally { restoreHostEnv(); @@ -545,6 +767,45 @@ suite('runtime metadata', () => { }); }); +suite('kernel probe', () => { + // The probe is what separates a kernel that is present from one that runs. + // The failure it exists for is a binary built against newer shared + // libraries than the host provides: exec succeeds, the dynamic linker then + // rejects it, and the process exits non-zero. + + test('a binary that exits non-zero does not pass', async function () { + if (process.platform === 'win32') { + this.skip(); + } + const dir = tempDir(); + const kernelPath = path.join(dir, binaryName); + fs.writeFileSync(kernelPath, '#!/bin/sh\nexit 1\n'); + fs.chmodSync(kernelPath, 0o755); + assert.strictEqual(await probeKernel(kernelPath), false); + }); + + test('a binary that exits zero passes', async function () { + if (process.platform === 'win32') { + this.skip(); + } + assert.strictEqual(await probeKernel(writeStubKernel(tempDir())), true); + }); + + test('a file that is not executable at all does not pass', async () => { + // The nearest reachable stand-in for a binary the loader rejects: the + // spawn fails rather than the process exiting non-zero, and the probe + // has to treat both the same way. + const kernelPath = path.join(tempDir(), binaryName); + fs.writeFileSync(kernelPath, 'not a real executable\n'); + fs.chmodSync(kernelPath, 0o644); + assert.strictEqual(await probeKernel(kernelPath), false); + }); + + test('a missing binary does not pass', async () => { + assert.strictEqual(await probeKernel(path.join(tempDir(), binaryName)), false); + }); +}); + suiteTeardown(() => { for (const dir of tempDirs) { fs.rmSync(dir, { recursive: true, force: true }); From 170a7fe01f774c0b1ba2018b819a5f543c3a5739 Mon Sep 17 00:00:00 2001 From: Sam Clark Date: Mon, 24 Aug 2026 11:04:18 -0500 Subject: [PATCH 7/8] Refine KernelStrategy type derivation to avoid unsafe casts Uses a const array with `(typeof)[number]` to derive the type and a type guard function, so the compiler enforces that KERNEL_STRATEGIES and KernelStrategy stay in sync instead of relying on manual `as` casts. --- ggsql-vscode/src/manager.ts | 12 ++++++++---- 1 file changed, 8 insertions(+), 4 deletions(-) diff --git a/ggsql-vscode/src/manager.ts b/ggsql-vscode/src/manager.ts index 360abf09..16a7fbb8 100644 --- a/ggsql-vscode/src/manager.ts +++ b/ggsql-vscode/src/manager.ts @@ -25,9 +25,13 @@ type KernelSource = 'Bundled' | 'Setting' | 'Jupyter' | 'System' | 'Path'; * bundled one. * - `path`: the binary named by `ggsql.kernelPath`. */ -export type KernelStrategy = 'bundled' | 'environment' | 'path'; +const KERNEL_STRATEGIES = ['bundled', 'environment', 'path'] as const; -const KERNEL_STRATEGIES: readonly string[] = ['bundled', 'environment', 'path']; +export type KernelStrategy = (typeof KERNEL_STRATEGIES)[number]; + +function isKernelStrategy(value: string): value is KernelStrategy { + return (KERNEL_STRATEGIES as readonly string[]).includes(value); +} /** * A discovered ggsql-jupyter kernel candidate @@ -199,8 +203,8 @@ export function resolveKernelStrategy(config: vscode.WorkspaceConfiguration): Ke ?? inspected?.globalValue; if (explicit !== undefined) { - if (KERNEL_STRATEGIES.includes(explicit)) { - return explicit as KernelStrategy; + if (isKernelStrategy(explicit)) { + return explicit; } log(`Ignoring unknown ggsql.kernelStrategy '${explicit}'`); } else if (config.get('kernelPath', '').trim() !== '') { From 86bccd8847e338a6b3419f3f30c374209209f849 Mon Sep 17 00:00:00 2001 From: Sam Clark Date: Mon, 24 Aug 2026 11:13:13 -0500 Subject: [PATCH 8/8] Resolve the kernel probe when spawn itself fails On Windows a file that is not a valid executable fails the CreateProcess call, which Node surfaces as a synchronous throw from execFile rather than a callback error. probeKernel only handled the callback path, so the promise rejected and discovery crashed instead of treating the kernel as unrunnable. Co-Authored-By: Claude Fable 5 --- ggsql-vscode/src/manager.ts | 30 +++++++++++++++++++----------- 1 file changed, 19 insertions(+), 11 deletions(-) diff --git a/ggsql-vscode/src/manager.ts b/ggsql-vscode/src/manager.ts index 16a7fbb8..85cfea47 100644 --- a/ggsql-vscode/src/manager.ts +++ b/ggsql-vscode/src/manager.ts @@ -385,17 +385,25 @@ export type KernelProbe = (kernelPath: string) => Promise; */ export function probeKernel(kernelPath: string): Promise { return new Promise(resolve => { - cp.execFile( - kernelPath, - ['--version'], - { timeout: KERNEL_PROBE_TIMEOUT_MS, windowsHide: true }, - err => { - if (err) { - log(`Kernel probe failed for ${kernelPath}: ${err.message}`); - } - resolve(!err); - }, - ); + // On Windows a file that is not a valid executable fails the + // CreateProcess call itself, which Node surfaces as a synchronous + // throw from execFile (`spawn UNKNOWN`) rather than a callback error. + try { + cp.execFile( + kernelPath, + ['--version'], + { timeout: KERNEL_PROBE_TIMEOUT_MS, windowsHide: true }, + err => { + if (err) { + log(`Kernel probe failed for ${kernelPath}: ${err.message}`); + } + resolve(!err); + }, + ); + } catch (err) { + log(`Kernel probe failed for ${kernelPath}: ${(err as Error).message}`); + resolve(false); + } }); }