diff --git a/CHANGELOG.md b/CHANGELOG.md index 8eefaed..4da5d07 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -85,6 +85,12 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 - **The MCP server is installed and started once when GemDB starts.** Opening a window on a running database while a cell started it could install the MCP server's classes twice and start two servers on one port. Now the second waits for the first. +- **Ctrl+C ends a `gemdb` run without an error.** Stopping `gemdb app.py` printed + `[Sending soft break]`, a line number in GemDB's own script, `ERROR 3004 , a ExitClientError + occurred` and `Logging out session 1.` around the `KeyboardInterrupt`, which read as a failure. It + now prints only `KeyboardInterrupt`, as Python does. GemStone's own messages from a `gemdb` run are + shown only when the program could not run to the end, such as a refused login, and a + `.topazini` file is no longer read. ## [1.6.0] - 2026-10-07 diff --git a/docs/grail.md b/docs/grail.md index 30440fb..cd6429a 100644 --- a/docs/grail.md +++ b/docs/grail.md @@ -63,15 +63,29 @@ longer stamps at all and the ordering question has gone with it. **The CLI's exit codes go through a status file, not topaz.** topaz cannot carry an exit status out of a `run` block — `ExitClientError status:` is not -translated, and an `iferr … exit 1` action exits 0 (all measured). The driver -therefore ends with no `exit` command at all, and must not grow one: `topaz -h` -says of `-S` that topaz "exits when the script completes" and that "exit and -quit commands are ignored". Ignored silently when stdin is a pipe — which is -every CI run, every test, and every `gemdb x.py | cat` — and out loud when -stdin is a tty, where it printed four lines of explanation and a -`Logging out session 1.` in front of the user. So +translated, and an `iferr … exit 1` action exits 0 (all measured). So `gemdb-run.tpz` writes the status to the file named in `GEMDB_STATUS_FILE` and -the bash wrapper becomes the exit code. Errors there are caught as +the bash wrapper becomes the exit code. + +**Topaz says nothing on the terminal during a file run.** The driver runs +under `-P` with an `-I` init file whose one line is +`output push $GEMDB_TOPAZ_LOG only`, so everything topaz prints goes to a +per-run log, while the program's output, written to `GsFile stdout` and +`stderr`, still reaches the terminal directly. Without it Ctrl-C ended a run +with a soft-break notice, `Near line 155 of file …/gemdb-run.tpz`, +`ERROR 3004` for the `ExitClientError` that ends topaz, and +`Logging out session 1.` (#131). All three pieces are needed, as measured on +4.0.0.a4. Under `-S`, `OUTPUT PUSH … ONLY` has no effect on stdout. Under `-P` +without `-I`, the lines before the push are echoed. And a log, not +`/dev/null`, because a refused login or a session the stone ends mid-run is +reported by topaz alone: the wrapper prints the log, less topaz's echo of the +driver, whenever no status was written. `-P` does not exit at the end of the +script, so the driver ends with `exit`. Without it, topaz reads its next +commands from stdin, and a refused login with input piped in ran that input as +topaz commands (measured, and covered by `cli.test.ts`). Under `-S` the same +`exit` was ignored, and on a tty it printed four lines saying so. + +Errors in the driver are caught as `AbstractException`, not `Error`: Grail's Python exceptions live outside the `Error` branch, which is why grail.tpz's own file mode exits 0 on a Python error. `sys.exit(n)` is decoded by that same handler: Grail raises its own diff --git a/src/__integration__/cli.test.ts b/src/__integration__/cli.test.ts index 3d3928b..06c247b 100644 --- a/src/__integration__/cli.test.ts +++ b/src/__integration__/cli.test.ts @@ -1,4 +1,4 @@ -import { execFile } from 'child_process'; +import { execFile, spawn } from 'child_process'; import * as fs from 'fs'; import * as os from 'os'; import * as path from 'path'; @@ -222,7 +222,7 @@ describe.skipIf(!ready || !canMakeFixture())('the gemdb command', () => { expect(ran.code).toBe(0); }); - it('stops on Ctrl-C, after finally blocks, rather than at a topaz prompt', async () => { + it('stops on Ctrl-C as python3 does, after finally blocks, with nothing from topaz', async () => { // A real Ctrl-C needs a real terminal: the line discipline turns ^C into // SIGINT for topaz, which sends the gem a soft break. expect(1) owns the // pty, as in repl.test.ts. The script blocks in a primitive, where a web @@ -268,12 +268,81 @@ exit [lindex $result 3] }, ); }); - expect(ran.transcript).toContain('cleaned up'); - expect(ran.transcript).toContain('KeyboardInterrupt'); - expect(ran.transcript).not.toContain('topaz 1>'); + // Everything the terminal showed after expect's own spawn line, less the + // ^C the line discipline echoes. Topaz used to add a soft-break notice, + // the driver line it stopped at, ERROR 3004 and a logout (#131). + const shown = ran.transcript + .replaceAll('^C', '') + .split(/\r?\n/) + .slice(1) + .map((line) => line.trim()) + .filter((line) => line !== ''); + + expect(shown).toEqual(['serving', 'cleaned up', 'KeyboardInterrupt']); expect(ran.code).toBe(130); }); + it('says why when the database refuses the login, and runs nothing piped to it', async () => { + const driver = path.join(path.dirname(cliPath()), 'gemdb-run.tpz'); + const good = fs.readFileSync(driver, 'utf8'); + fs.writeFileSync(driver, good.replace(/^(set user \S+ pass )\S*$/m, '$1not-the-password')); + let ran: Ran; + try { + ran = await run(['hello.py'], 'shell echo LEAKED\n'); + } finally { + fs.writeFileSync(driver, good); + } + + // Exactly GemStone's report of the refusal: none of the driver, which + // topaz echoes into the log and the wrapper strips, nothing about the run + // block topaz goes on through with no session, and not the piped line. + // stdout is not pinned: the gem sometimes writes a line of its own there + // about the failed login, which is not topaz's to keep quiet. + expect(ran.stderr.trimEnd().split('\n')).toEqual([ + expect.stringMatching(/^-+$/), + 'GemStone: Error Fatal', + 'Login failed: the userId/password combination is invalid or expired.', + expect.stringContaining('Number: 4051'), + 'Arg 1: nil', + ]); + expect(ran.stdout).not.toContain('LEAKED'); + expect(ran.stdout).not.toContain('hello from'); + // topaz's own status, which the wrapper passes on when there is no other. + expect(ran.code).toBe(1); + }); + + it('leaves no temporary files behind, even when its terminal closes mid-run', async () => { + const tmp = fs.mkdtempSync(path.join(os.tmpdir(), 'gemdb-cli-tmp-')); + const env = { ...process.env, TMPDIR: tmp }; + fs.writeFileSync( + path.join(workDir, 'forever.py'), + 'import time\nprint("serving", flush=True)\nwhile True:\n time.sleep(0.2)\n', + ); + await new Promise((resolve) => + execFile('bash', [cliPath(), '-c', 'print(1)'], { cwd: workDir, env }, resolve), + ); + // Its own process group, so the hangup reaches the wrapper and topaz + // together, as closing a terminal tab does. + const child = spawn('bash', [cliPath(), 'forever.py'], { cwd: workDir, env, detached: true }); + const exited = new Promise((resolve) => child.on('exit', resolve)); + await new Promise((resolve) => { + child.stdout.on('data', (chunk: Buffer) => { + if (chunk.toString().includes('serving')) resolve(); + }); + }); + + process.kill(-(child.pid ?? 0), 'SIGHUP'); + await exited; + const left = fs.readdirSync(tmp); + // topaz outlives a hangup, as it did before the wrapper had a trap, so it + // is ended here: left running it holds a session the next test needs gone. + process.kill(-(child.pid ?? 0), 'SIGTERM'); + await waitForGroupToEnd(child.pid ?? 0); + fs.rmSync(tmp, { recursive: true, force: true }); + + expect(left).toEqual([]); + }); + it('raises EOFError when stdin runs dry', async () => { const ran = await run(['-c', 'input()'], ''); expect(ran.stderr).toContain('EOF'); @@ -316,3 +385,16 @@ function canMakeFixture(): boolean { probe?.remove(); return probe !== undefined; } + +/** Resolves once no process in the group is left, polling as `kill -0` does. */ +async function waitForGroupToEnd(pgid: number): Promise { + for (let i = 0; i < 100; i++) { + try { + process.kill(-pgid, 0); + } catch { + return; + } + await new Promise((resolve) => setTimeout(resolve, 100)); + } + throw new Error(`process group ${pgid} still running after 10 s`); +} diff --git a/src/__tests__/cli.test.ts b/src/__tests__/cli.test.ts index 6439938..99e8a13 100644 --- a/src/__tests__/cli.test.ts +++ b/src/__tests__/cli.test.ts @@ -77,19 +77,13 @@ describe('writeCliScripts', () => { // only path. Sending it is now a doesNotUnderstand that kills the run at // that line, so the driver must not carry it. expect(run).not.toContain('___canonicalClassesEnabled___'); - // A file run is linked topaz, so it never reaches session.ts and would - // otherwise sit in the shared cache as the stock 'TopazL'. The truncation - // is not optional: 32 characters raises OutOfRange, at login. - // No `exit` command: with -S, topaz exits when the script completes and - // ignores exit outright — quietly on a pipe, but on a tty it prints four - // lines about ignoring it and a spurious "Logging out session 1.". A test - // here because a pipe is what both suites use, so nothing else would - // notice it coming back. - expect(run.split('\n').some((line) => /^\s*(exit|quit)\b/.test(line))).toBe(false); // The console sink is a GsFile, which takes bytes; the second slot of // the #GrailConsole box is what tells Grail to encode. Without it, // non-ASCII print() output is UTF-16 code units on the terminal. expect(run).toContain("put: (Array with: GsFile stdout with: #'utf8')"); + // A file run is linked topaz, so it never reaches session.ts and would + // otherwise sit in the shared cache as the stock 'TopazL'. The truncation + // is not optional: 32 characters raises OutOfRange, at login. expect(run).toContain('System cacheName: label'); expect(run).toContain("label := 'GemDB run ', label"); expect(run).toContain('label size > 31 ifTrue:'); @@ -113,6 +107,47 @@ describe('writeCliScripts', () => { ); }); + it("keeps topaz's own output off the terminal, and shows it when a run never finished", () => { + writeCliScripts(ext); + + const script = fs.readFileSync(cliPath(), 'utf8'); + const init = fs.readFileSync(path.join(root, 'bin', 'gemdb-run-init.tpz'), 'utf8'); + + // Only -P honours OUTPUT PUSH ... ONLY for stdout, and only an -I file is + // not echoed before the push takes effect. + expect(init).toBe('output push $GEMDB_TOPAZ_LOG only\n'); + expect(script).toContain( + 'topaz -L -q -I "$ROOT/bin/gemdb-run-init.tpz" -P "$ROOT/bin/gemdb-run.tpz"', + ); + expect(script).toContain('GEMDB_TOPAZ_LOG="$(mktemp'); + expect(script).toMatch(/if \[ -z "\$STATUS" \]; then\n\s+sed .* "\$GEMDB_TOPAZ_LOG" >&2\n/); + }); + + it('removes its temporary files however the run ends', () => { + writeCliScripts(ext); + + const script = fs.readFileSync(cliPath(), 'utf8'); + + // Armed before the first file is made, so no exit path — a hangup + // included — can skip it, and nothing after the run removes them itself. + expect(script.indexOf('trap remove_temp_files EXIT')).toBeGreaterThan(-1); + expect(script.indexOf('trap remove_temp_files EXIT')).toBeLessThan(script.indexOf('mktemp')); + expect(script).not.toMatch(/^rm -f "\$GEMDB_STATUS_FILE"/m); + }); + + it('never leaves topaz reading commands from stdin', () => { + writeCliScripts(ext); + + const lines = fs + .readFileSync(path.join(root, 'bin', 'gemdb-run.tpz'), 'utf8') + .trimEnd() + .split('\n'); + + // Under -P, topaz reads stdin once the script ends — a terminal, or + // whatever was piped to the user's program. + expect(lines[lines.length - 1]).toBe('exit'); + }); + it('reports an uncaught exception only after the stack has unwound', () => { writeCliScripts(ext); diff --git a/src/cli.ts b/src/cli.ts index 5850490..7f7da97 100644 --- a/src/cli.ts +++ b/src/cli.ts @@ -26,11 +26,14 @@ import { sharedLibraryExtension } from './platform'; * Generated files under `/bin`, rewritten on every staging so they * always match the installed engine and the staged Grail: * - * `gemdb` the wrapper. Sets the whole engine environment itself, - * so it works from any shell with no setup beyond PATH. - * `gemdb-run.tpz` the topaz driver for running a file or module. - * `gemdb-shell.js` the GemDB Shell, staged out of the extension's build - * together with `node_modules/koffi` beside it. + * `gemdb` the wrapper. Sets the whole engine environment + * itself, so it works from any shell with no setup + * beyond PATH. + * `gemdb-run.tpz` the topaz driver for running a file or module. + * `gemdb-run-init.tpz` its init file, which keeps topaz's own output off + * the terminal (see TOPAZ_INIT below). + * `gemdb-shell.js` the GemDB Shell, staged out of the extension's + * build together with `node_modules/koffi` beside it. * * File mode drives *linked* topaz — the right login for a batch run: its own * process, one session, crash isolation by construction, and real stdin/stdout @@ -68,6 +71,45 @@ export function cliPath(): string { const TOPAZ_TUNING = `-T 400000 -C 'GEM_TEMPOBJ_CODE_SIZE=300000;'`; +/** + * The init file (`-I`) for a file run: everything topaz itself says goes to a + * per-run log the wrapper names in GEMDB_TOPAZ_LOG, and never to the terminal. + * + * The program's own output is unaffected, because it never passes through + * topaz: print() and the driver's reports write to `GsFile stdout`/`stderr`, + * which in linked topaz are the process's real descriptors. What this removes + * is topaz talking over the program. Ctrl-C used to end a run with + * + * [Sending soft break] + * Near line 155 of file …/gemdb-run.tpz + * KeyboardInterrupt + * ERROR 3004 , a ExitClientError occurred (error 3004) + * Logging out session 1. + * + * where CPython prints only the KeyboardInterrupt — and the ERROR line, which + * is the driver's deliberate way out (see the end of the driver), read as a + * failure (GemTalk/GemDB_Code#131). Each piece here was measured on 4.0.0.a4: + * + * - **`-P`, not `-S`.** Under `-S`, `OUTPUT PUSH … ONLY` has no effect on + * stdout (topaz's own help says so, and `output push /dev/null only` changed + * nothing). Under `-P` it takes effect, the soft-break notice on stderr + * included. + * - **Pushed here, not in the driver.** `-P` echoes each script line to the + * terminal until the push takes effect, so a push on the driver's first line + * still printed that line and the one before it. An `-I` file is not echoed. + * `-i` is no substitute: it did not stop `-P`'s echo, and it ignores `-I`. + * - **A log, not /dev/null.** A refused login (the session limit, the space + * threshold, an external database's password) or a session the stone ends + * mid-run is reported by topaz and nothing else, so the wrapper shows the + * log whenever the run did not report a status. + * + * `-I` also means no `.topazini` is read, from home or from the current + * directory — which the driver's `set` lines already overrode for the login, + * and which a cloned repository could otherwise use to run topaz commands. + */ +const TOPAZ_INIT = `output push $GEMDB_TOPAZ_LOG only +`; + /** * Copy the shell bundle and its one native dependency beside the wrapper. * @@ -118,6 +160,7 @@ function cliFingerprint(wrapper: string, driver: string, shellBundle: string): s const hash = crypto.createHash('sha256'); hash.update(wrapper); hash.update(driver); + hash.update(TOPAZ_INIT); hash.update(fs.existsSync(shellBundle) ? fs.readFileSync(shellBundle) : Buffer.of()); return hash.digest('hex'); } @@ -196,22 +239,23 @@ export function writeCliScripts(extensionPath: string): void { const external = externalDatabase(); const stone = stoneName(); - // 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 — - // Grail's Python exceptions live outside the Error branch, which is why - // grail.tpz's own file mode exits 0 on a Python error. + // The topaz driver for one file or module, run with `-P` after TOPAZ_INIT. + // No ~/.topazini runs (`-I` replaces it), and the `set` lines would beat one + // anyway, so a Jasper user's defaults cannot hijack the login. Errors are + // caught as AbstractException, not Error — Grail's Python exceptions live + // outside the Error branch, which is why grail.tpz's own file mode exits 0 + // on a Python error. // - // There is deliberately no `exit` command at the end. topaz's own help says - // it of `-S`: the script is processed with INPUT, "topaz exits when the - // script completes", and "exit and quit commands are ignored". Ignored - // quietly when stdin is a pipe, which is why both suites and every run - // through a non-interactive shell missed it — but out loud when stdin is a - // tty, where `gemdb hide.py` printed four lines of topaz explaining that it - // was ignoring the EXIT, plus a "Logging out session 1." the ignored exit - // provoked. The status still leaves through GEMDB_STATUS_FILE, which never - // depended on that line (measured again without it: sys.exit(3) → 3, an - // uncaught exception → 1, a missing file → 2). + // `-P` is why the driver ends with `exit`. Unlike `-S`, it does not exit at + // the end of the script: it goes on to read commands from stdin, which is + // the user's terminal or, worse, whatever was piped to their program. A + // topaz-level error does not change that — measured with a wrong password, + // topaz reports the refused login, carries on through the script, and + // leaves at this line, on a tty and on a pipe alike. Only Ctrl-C stops it + // reading, and the ExitClientError below covers that. (Under `-S` the + // closing exit was the opposite problem: ignored, and on a tty four lines + // saying so.) The status still leaves through GEMDB_STATUS_FILE, not the + // exit: sys.exit(3) → 3, an uncaught exception → 1, a missing file → 2. // // sys.exit(n) is decoded here, not upstream: Grail raises its own SystemExit // (never ExitClientError — input()'s except SystemExit and finally blocks @@ -382,9 +426,12 @@ sys.breakpointhook = _gemdb_breakpoint']] and it stops reading this script once the block returns, leaving the user at a topaz 1> prompt still holding a session -- so nothing written after this block would run. An ExitClientError is what makes topaz log out and exit -instead. The status still travels through GEMDB_STATUS_FILE, written above." +instead. The status still travels through GEMDB_STATUS_FILE, written above, +and topaz's ERROR 3004 for it goes to the log TOPAZ_INIT opened, which the +wrapper shows only when there is no status." interrupted ifTrue: [ExitClientError new status: status; signal]. % +exit `; const script = `#!/bin/bash @@ -577,9 +624,24 @@ fi fi fi +# The run's temporary files, removed however it ends: bash runs an EXIT trap +# on a normal exit, and on SIGHUP (a terminal tab closed mid-run) and SIGTERM +# too — measured with macOS's /bin/bash 3.2, the oldest this runs under. A +# function rather than an array, because 3.2 calls an empty array unbound +# under set -u. +CODE_FILE="" +GEMDB_STATUS_FILE="" +GEMDB_TOPAZ_LOG="" +remove_temp_files() { + local f + for f in "$CODE_FILE" "\${CODE_FILE%.py}" "$GEMDB_STATUS_FILE" "$GEMDB_TOPAZ_LOG"; do + if [ -n "$f" ]; then rm -f "$f"; fi + done +} +trap remove_temp_files EXIT + # -c 'code': materialise the code as a file, like CPython's -c but visibly so. # mktemp is given an explicit template — BSD and GNU disagree about -t. -CODE_FILE="" if [ "$1" = "-c" ]; then [ "$#" -ge 2 ] || { echo "gemdb: -c requires an argument" >&2; exit 2; } CODE_FILE="$(mktemp "\${TMPDIR:-/tmp}/gemdb-c-XXXXXX").py" @@ -597,13 +659,20 @@ fi # topaz cannot carry an exit status out of a run block, so the driver writes # it to a file and this wrapper becomes the exit code. See gemdb-run.tpz. +# Everything topaz itself prints goes to GEMDB_TOPAZ_LOG instead of the +# terminal (gemdb-run-init.tpz), so that Ctrl-C ends a program the way it ends +# CPython. No status means the program never finished — a refused login, or a +# session ended under it — and then topaz's account is the only one, so it is +# shown, less topaz's echo of the driver. GEMDB_STATUS_FILE="$(mktemp "\${TMPDIR:-/tmp}/gemdb-status-XXXXXX")" -export GEMDB_STATUS_FILE -topaz -L -q -S "$ROOT/bin/gemdb-run.tpz" ${TOPAZ_TUNING} -- "$@" +GEMDB_TOPAZ_LOG="$(mktemp "\${TMPDIR:-/tmp}/gemdb-topaz-XXXXXX")" +export GEMDB_STATUS_FILE GEMDB_TOPAZ_LOG +topaz -L -q -I "$ROOT/bin/gemdb-run-init.tpz" -P "$ROOT/bin/gemdb-run.tpz" ${TOPAZ_TUNING} -- "$@" RC=$? STATUS="$(cat "$GEMDB_STATUS_FILE" 2>/dev/null || true)" -rm -f "$GEMDB_STATUS_FILE" -[ -n "$CODE_FILE" ] && rm -f "$CODE_FILE" "\${CODE_FILE%.py}" +if [ -z "$STATUS" ]; then + sed -e '/^topaz[^>]*> run$/,/^%$/d' -e '/^topaz[^>]*> /d' "$GEMDB_TOPAZ_LOG" >&2 +fi case "$STATUS" in ''|*[!0-9]*) [ "$RC" -ne 0 ] && exit "$RC"; exit 1 ;; *) exit "$STATUS" ;; @@ -631,6 +700,7 @@ esac const driverPath = path.join(cliDirPath(), 'gemdb-run.tpz'); fs.writeFileSync(driverPath, driver, { mode: 0o600 }); fs.chmodSync(driverPath, 0o600); + fs.writeFileSync(path.join(cliDirPath(), 'gemdb-run-init.tpz'), TOPAZ_INIT); // 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.