diff --git a/CHANGELOG.md b/CHANGELOG.md index 68b1ae3..a7d9666 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -7,6 +7,24 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ## [Unreleased] +### Fixed + +- **The GemDB Shell no longer prints the database client's own chatter.** + Opening a shell wrote a line like + `gcits login: session 0x… lgc 0x… rpc gem processId 4726` onto your terminal + between the banner and the first prompt, with a matching one on exit — the + client library narrating itself. Every session GemDB opens now asks it not + to. +- **Updating GemDB now updates the `gemdb` command and the shell it runs.** + The staged copies under `~/GemDB/bin` were rewritten only when the Python + payload changed, so a release that changed only extension code left the + previous ones in place — a fix to the GemDB Shell reached the editor while + the terminal it opens kept running the old build. They are now refreshed + whenever they differ from what the installed version carries, and left alone + when they do not. Opening a GemDB Shell or running a file now guarantees the + command is there and current first, rather than handing VS Code a path and + letting it report "The terminal process failed to launch". + ## [1.2.0] - 2026-08-27 A notebook is a unit of work again: each one gets its own database session, and diff --git a/src/__integration__/repl.test.ts b/src/__integration__/repl.test.ts index 034e3c2..073c59d 100644 --- a/src/__integration__/repl.test.ts +++ b/src/__integration__/repl.test.ts @@ -162,6 +162,11 @@ send "exit()\\r" expect(ran.transcript).toContain('42'); expect(ran.transcript).toContain('marco polo'); expect(ran.transcript).toContain('KeyboardInterrupt'); + // The client library narrates every login and logout on stdout unless the + // session asks it not to (GCI_LOGIN_QUIET, session.ts). This is the one + // place that shows: the shell's stdout is the user's terminal, so the + // chatter would land between the banner and the first prompt. + expect(ran.transcript).not.toContain('gcits'); expect(ran.code).toBe(0); expect(isRunning()).toBe(true); // the shell brought the stone up itself }); diff --git a/src/__tests__/cli.test.ts b/src/__tests__/cli.test.ts index d01ae0c..d054655 100644 --- a/src/__tests__/cli.test.ts +++ b/src/__tests__/cli.test.ts @@ -3,8 +3,8 @@ import * as os from 'os'; import * as path from 'path'; import { afterEach, beforeEach, describe, expect, it } from 'vitest'; import { __setSetting } from '../__mocks__/vscode'; -import { cliPath, writeCliScripts } from '../cli'; -import { expectedEnginePath } from '../paths'; +import { cliPath, ensureCliCurrent, writeCliScripts } from '../cli'; +import { cliStampPath, expectedEnginePath } from '../paths'; /** * The generator, not the command: what the files say and where they land. @@ -132,6 +132,68 @@ describe('writeCliScripts', () => { expect(fs.existsSync(path.join(root, 'bin', 'gemdb-repl.tpz'))).toBe(false); }); + it('restages when the shell bundle changed, even though nothing else did', () => { + // The bug this guards: staging used to run only when the Grail payload + // changed, so an update carrying only code left the previous bundle in + // place. A fix to the shell reached the editor and not the terminal it + // opens — which is exactly how a stale `gcits login:` line survived a + // release that had silenced it. + writeCliScripts(ext); + const staged = path.join(root, 'bin', 'gemdb-shell.js'); + expect(fs.readFileSync(staged, 'utf8')).toBe('// the shell bundle\n'); + + fs.writeFileSync(path.join(ext, 'out', 'gemdb-shell.js'), '// rebuilt\n'); + writeCliScripts(ext); + + expect(fs.readFileSync(staged, 'utf8')).toBe('// rebuilt\n'); + }); + + it('writes nothing when it would write the same bytes', () => { + // Called on every path to a running database, so the common case has to + // be cheap: no rewrite, and above all no rm/copy of koffi. + writeCliScripts(ext); + const before = fs.statSync(cliPath()).mtimeMs; + const stamp = fs.readFileSync(cliStampPath(), 'utf8'); + + writeCliScripts(ext); + + expect(fs.statSync(cliPath()).mtimeMs).toBe(before); + expect(fs.readFileSync(cliStampPath(), 'utf8')).toBe(stamp); + }); + + it('restages when the wrapper itself would differ', () => { + // The wrapper bakes in paths and the editor's own Node runtime, so it goes + // stale for reasons that have nothing to do with the bundle. + writeCliScripts(ext); + fs.writeFileSync(cliPath(), '#!/bin/bash\n# tampered\n'); + fs.writeFileSync(cliStampPath(), 'not-the-fingerprint\n'); + + writeCliScripts(ext); + + expect(fs.readFileSync(cliPath(), 'utf8')).toContain(`ROOT="${root}"`); + }); + + it('leaves no stamp claiming success when generation failed', () => { + // The stamp is a claim that everything above it was written. + fs.rmSync(expectedEnginePath(), { recursive: true, force: true }); + expect(() => writeCliScripts(ext)).toThrow(/not installed/); + expect(fs.existsSync(cliStampPath())).toBe(false); + }); + + it('reports whether the command a terminal is about to launch is really there', () => { + // openRepl hands this path to VS Code as a terminal's shell program, so a + // false here is the difference between an explanation and + // "The terminal process failed to launch". + expect(ensureCliCurrent(ext)).toBe(true); + expect(fs.existsSync(cliPath())).toBe(true); + + // Generation impossible: it must say so rather than throw, and must not + // claim a wrapper that is not there. + fs.rmSync(cliPath()); + fs.rmSync(expectedEnginePath(), { recursive: true, force: true }); + expect(ensureCliCurrent(ext)).toBe(false); + }); + it('refuses to generate against a missing engine', () => { fs.rmSync(expectedEnginePath(), { recursive: true, force: true }); expect(() => writeCliScripts(ext)).toThrow(/not installed/); diff --git a/src/cli.ts b/src/cli.ts index e1ec99f..247337b 100644 --- a/src/cli.ts +++ b/src/cli.ts @@ -1,3 +1,4 @@ +import * as crypto from 'crypto'; import * as fs from 'fs'; import * as path from 'path'; import { @@ -9,7 +10,7 @@ import { rootPath, } from './config'; import { log } from './log'; -import { enginePath, grailPath } from './paths'; +import { cliStampPath, enginePath, grailPath } from './paths'; import { sharedLibraryExtension } from './platform'; /** @@ -103,14 +104,61 @@ function stageShell(extensionPath: string): void { }); } +/** + * What is staged, in one line, so staging can tell whether it has work to do. + * + * Content rather than a version number, because the thing that goes stale is + * content: `out/gemdb-shell.js` is rebuilt on every `npm run bundle`, and the + * wrapper bakes in the editor's own `process.execPath`, which moves when VS + * Code updates. A version stamp would miss both. + */ +function cliFingerprint(wrapper: string, driver: string, shellBundle: string): string { + const hash = crypto.createHash('sha256'); + hash.update(wrapper); + hash.update(driver); + hash.update(fs.existsSync(shellBundle) ? fs.readFileSync(shellBundle) : Buffer.of()); + return hash.digest('hex'); +} + +/** Is what is on disk already exactly what this call would write? */ +function cliIsCurrent(fingerprint: string): boolean { + if (!fs.existsSync(cliPath())) return false; + try { + return fs.readFileSync(cliStampPath(), 'utf8').trim() === fingerprint; + } catch { + return false; + } +} + +/** + * Guarantee `/bin/gemdb` exists and matches this build, for callers + * about to hand that path to something else. + * + * Opening a GemDB Shell makes VS Code launch the wrapper as a terminal's shell + * program, so a missing or stale file is not an internal detail — it is + * "The terminal process failed to launch", or a shell running last week's + * code. `requireRunning` cannot carry this: it returns early when the database + * is already up, which is the common case, so `ensureRunning` — where staging + * lives — is skipped exactly when nothing else would notice. + * + * Cheap to call: `writeCliScripts` fingerprints what it would write and + * returns without touching the disk when that matches what is already there. + */ +export function ensureCliCurrent(extensionPath: string): boolean { + try { + writeCliScripts(extensionPath); + } catch (e) { + log(`Could not write the gemdb command: ${e instanceof Error ? e.message : e}`); + } + return fs.existsSync(cliPath()); +} + export function writeCliScripts(extensionPath: string): void { const engine = enginePath(); if (!engine) throw new Error('The database engine is not installed.'); const root = rootPath(); const grail = grailPath(); - fs.mkdirSync(cliDirPath(), { recursive: true }); - // The topaz driver for one file or module. `set` lines beat any ~/.topazini // (the ini runs first, these run later), so a Jasper user's defaults cannot // hijack the login. Errors are caught as AbstractException, not Error — @@ -125,9 +173,7 @@ export function writeCliScripts(extensionPath: string): void { // CPython's contract, each case measured: None or no argument exits 0 // silently, an int exits `n % 256` silently (-1 → 255, 256 → 0), and // anything else prints str(code) to stderr and exits 1. - fs.writeFileSync( - path.join(cliDirPath(), 'gemdb-run.tpz'), - `! Generated by GemDB. Regenerated on every update — do not edit. + const driver = `! Generated by GemDB. Regenerated on every update — do not edit. set user ${DB_USER} pass ${DB_PASSWORD} set gemstone ${STONE_NAME} login @@ -214,14 +260,7 @@ SessionTemps current at: #'GrailConsole' put: (Array with: GsFile stdout). ]. % exit 0 -`, - ); - - // An earlier release handed the no-argument mode to Grail's topaz REPL - // through this file; the shell replaced it, so a stale copy is only a trap. - fs.rmSync(path.join(cliDirPath(), 'gemdb-repl.tpz'), { force: true }); - - stageShell(extensionPath); +`; const script = `#!/bin/bash # gemdb — run Python inside the GemDB database, from any shell. @@ -324,7 +363,33 @@ case "$STATUS" in esac `; + // Nothing to do when what is on disk is already exactly this. The check + // exists for the opposite case: staging used to happen only when the Grail + // payload changed, so an extension update that changed only code left the + // previous `bin/gemdb` and shell bundle in place — a fix to the shell would + // reach the editor and not the terminal it opens. + const fingerprint = cliFingerprint( + script, + driver, + path.join(extensionPath, 'out', 'gemdb-shell.js'), + ); + if (cliIsCurrent(fingerprint)) { + log('The gemdb command is already current.'); + return; + } + + fs.mkdirSync(cliDirPath(), { recursive: true }); + fs.writeFileSync(path.join(cliDirPath(), 'gemdb-run.tpz'), driver); + + // An earlier release handed the no-argument mode to Grail's topaz REPL + // through this file; the shell replaced it, so a stale copy is only a trap. + fs.rmSync(path.join(cliDirPath(), 'gemdb-repl.tpz'), { force: true }); + + stageShell(extensionPath); + fs.writeFileSync(cliPath(), script); fs.chmodSync(cliPath(), 0o755); + // Written last: a stamp is a claim that everything above succeeded. + fs.writeFileSync(cliStampPath(), `${fingerprint}\n`); log(`Wrote the gemdb command to ${cliPath()}`); } diff --git a/src/lifecycle.ts b/src/lifecycle.ts index 0e9eeed..7d46e39 100644 --- a/src/lifecycle.ts +++ b/src/lifecycle.ts @@ -1,7 +1,7 @@ import * as fs from 'fs'; import * as vscode from 'vscode'; import { engineVersion, reinstallPythonOnUpdate, rootPath } from './config'; -import { cliPath, writeCliScripts } from './cli'; +import { writeCliScripts } from './cli'; import { createDatabase, removeDatabase } from './database'; import { Progress, installEngine, removeEngine } from './engine'; import { @@ -278,16 +278,17 @@ export async function ensureRunning(extensionPath: string): Promise { if (!prepared || !isInstalled()) return false; } - // The shell command is normally written when Grail is staged, but an - // install that predates it never runs that staging again — the payload on - // disk already matches. Backstop it here, on the single path everything - // that needs a database goes through. - if (!fs.existsSync(cliPath())) { - try { - writeCliScripts(extensionPath); - } catch (e) { - log(`Could not write the gemdb command: ${errorMessage(e)}`); - } + // Staging Grail writes the shell command too, but only when the payload + // changed — so an update that changed only code would leave the previous + // `bin/gemdb` and shell bundle in place, and a fix to the shell would reach + // the editor while the terminal it opens kept running the old one. Called + // unconditionally here, on the single path everything that needs a database + // goes through; `writeCliScripts` compares a fingerprint and returns without + // touching the disk when it would write the same bytes. + try { + writeCliScripts(extensionPath); + } catch (e) { + log(`Could not write the gemdb command: ${errorMessage(e)}`); } if (!(await ensureOsConfigured(extensionPath))) return false; diff --git a/src/paths.ts b/src/paths.ts index 1ff3f1c..d2a588a 100644 --- a/src/paths.ts +++ b/src/paths.ts @@ -76,6 +76,20 @@ export function grailStampPath(): string { return path.join(grailPath(), '.gemdb-grail-stamp'); } +/** + * Marker recording which build of the generated `gemdb` command is staged. + * + * Holds a fingerprint of what `writeCliScripts` would produce — the wrapper, + * the topaz driver and the shell bundle — so staging can be skipped when it + * would rewrite the same bytes, and, more to the point, is NOT skipped when it + * would not. Its own stamp rather than the extension's version because a + * developer running the extension host rebuilds the bundle far more often than + * they change the version. + */ +export function cliStampPath(): string { + return path.join(rootPath(), 'bin', '.gemdb-cli-stamp'); +} + export function locksPath(): string { return path.join(rootPath(), 'locks'); } diff --git a/src/repl.ts b/src/repl.ts index e8d3092..022740b 100644 --- a/src/repl.ts +++ b/src/repl.ts @@ -3,7 +3,7 @@ import * as vscode from 'vscode'; import { shellQuote } from './osConfig'; import { findNetldi, findStone } from './processes'; import { ensureRunning } from './lifecycle'; -import { cliPath } from './cli'; +import { cliPath, ensureCliCurrent } from './cli'; /** * Make sure the database is up, starting it if it is not. @@ -29,6 +29,15 @@ async function requireRunning(extensionPath: string): Promise { let replCounter = 0; export async function openRepl(extensionPath: string): Promise { if (!(await requireRunning(extensionPath))) return; + // The wrapper is this terminal's shell program, so it has to be there and + // has to be this build — VS Code reports a missing one as a launch failure + // with no hint of what GemDB should have done about it. + if (!ensureCliCurrent(extensionPath)) { + void vscode.window.showErrorMessage( + 'GemDB could not write the gemdb command, so the GemDB Shell cannot start. See the GemDB log.', + ); + return; + } replCounter += 1; const name = replCounter === 1 ? 'GemDB Shell' : `GemDB Shell ${replCounter}`; @@ -62,6 +71,14 @@ export async function runFile(extensionPath: string, uri?: vscode.Uri): Promise< return; } if (!(await requireRunning(extensionPath))) return; + // Same guarantee as the shell: this terminal is about to be sent the + // wrapper's path as a command line. + if (!ensureCliCurrent(extensionPath)) { + void vscode.window.showErrorMessage( + 'GemDB could not write the gemdb command, so the file cannot be run. See the GemDB log.', + ); + return; + } // Save first: the database reads the file from disk, so an unsaved buffer // would silently run the previous version. diff --git a/src/session.ts b/src/session.ts index b495a1e..663ce95 100644 --- a/src/session.ts +++ b/src/session.ts @@ -48,6 +48,23 @@ export class SessionLimitError extends SessionError {} */ const SESSION_LIMIT_ERRORS = new Set([4039, 4041, 4050]); +/** + * `GCI_LOGIN_QUIET` — stop the client library narrating each login on stdout. + * + * Without it every login writes a line like + * `gcits login: session 0x… lgc 0x… rpc gem processId 4726` (and a matching + * one at logout) to the process's real stdout, from inside the C library. In + * the extension host that is merely noise in the log; in a GemDB Shell it is + * printed straight onto the user's terminal, and in `gemdb -c` it lands in + * output a script may be piping somewhere. + * + * Defined here rather than in `gci/gciConstants.ts` because that directory is + * vendored from Jasper byte-for-byte and carries no login flags at all. The + * value is from the engine's own `include/gci.ht` (`GCI_LOGIN_QUIET = 0x10`, + * in the flag enum `GciTsLogin`'s `loginFlags` takes). + */ +const GCI_LOGIN_QUIET = 0x10; + /** What kind of user interface a session belongs to. */ export type SessionKind = 'notebook' | 'shell' | 'extension'; @@ -400,7 +417,7 @@ export class GciSession { gemNrs(), DB_USER, DB_PASSWORD, - 0, + GCI_LOGIN_QUIET, 0, ); if (!result.session) {