Skip to content

Keep topaz's own output off the terminal during a gemdb run - #132

Merged
jgfoster merged 2 commits into
mainfrom
james/quiet-ctrl-c
Oct 15, 2026
Merged

jgfoster merged 2 commits into
mainfrom
james/quiet-ctrl-c

Conversation

@jgfoster

Copy link
Copy Markdown
Member

Fixes #131.

What a reader sees

Stopping gemdb app.py with Ctrl+C, before:

^C
[Sending soft break]
 Near line 155 of file …/bin/gemdb-run.tpz
cleaned up
KeyboardInterrupt
ERROR 3004 , a ExitClientError occurred (error 3004)
Logging out session 1.

After (the ^C is the terminal's own echo, as with python3):

^Ccleaned up
KeyboardInterrupt

The exit status is still 130.

How

Everything topaz itself prints now goes to a per-run log instead of the terminal. The program's output is unaffected because it never goes through topaz: print() and the driver's own reports write to GsFile stdout/stderr, which in linked topaz are the process's real descriptors.

Three pieces, each needed (measured on 4.0.0.a4):

  • -P instead of -S. Under -S, OUTPUT PUSH … ONLY has no effect on stdout. Topaz's help says so, and output push /dev/null only changed nothing.
  • The push lives in an -I init file (bin/gemdb-run-init.tpz). -P echoes script lines until the push takes effect. -i doesn't stop that echo.
  • A log, not /dev/null. A refused login (session limit, space threshold, an external database's password) or a session the stone ends mid-run is reported only by topaz. When no status came back, the wrapper prints the log, minus topaz's echo of the driver.

Two consequences:

  • The driver now ends with exit. Under -P, topaz reads commands from stdin at the end of the script. Without the exit, a refused login with input piped in ran that input as topaz commands; the new integration test catches that if the exit is removed.
  • No .topazini is read any more, whether from home or from the current directory. The driver's set lines already overrode one for the login.

Approaches tried that didn't work: System logout instead of the ExitClientError (worse: a fatal-error block, then stranded at topaz>), and GsFile _getStdFile: 3 (only descriptors 0–2 are accepted).

Tests

  • cli.test.ts (integration): the Ctrl-C test now requires the terminal to show exactly serving, cleaned up, KeyboardInterrupt. New test: a refused login is reported on stderr, the password stays redacted, and input piped in is not run.
  • cli.test.ts (unit): the old "no exit line" assertion is inverted, with checks on the init file and the wrapper.
  • Also checked by hand under a pty: Ctrl-C while input() waits, a double Ctrl-C, input() and sys.exit(3), an uncaught exception, and a refused login on a terminal.
  • Lint, format, both typechecks, unit suite (900), and the full integration suite (138 passed, plus the existing expected-fail and todo) pass on darwin-arm64. The Linux legs are for CI.

🤖 Generated with Claude Code

Ctrl+C ended `gemdb app.py` with `[Sending soft break]`, a line number in
gemdb-run.tpz, `ERROR 3004 , a ExitClientError occurred` and `Logging out
session 1.` around the KeyboardInterrupt, and the ERROR line read as a
failure. The wrapper now runs the driver under -P with an -I init file
that pushes topaz's output to a per-run log, so the terminal shows only
what the program printed. The program's output is untouched: it goes to
GsFile stdout/stderr, the process's own descriptors.

The log is shown, less topaz's echo of the driver, when no status came
back: a refused login or a session ended mid-run is reported by topaz
alone. Under -P topaz reads stdin at the end of the script, so the driver
now ends with `exit`; without it, a refused login with input piped in ran
that input as topaz commands. -I also stops any .topazini being read.

Fixes #131.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Comment thread src/cli.ts
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")"

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💭 Should we clean these temp files up with a trap '…' EXIT instead of relying on the rm -f further down?

Under -P the log also gets topaz's echo of set user … pass … (which is what the redaction assertion in the new integration test guards against on stderr), so the log now holds the password at rest — for an external database, that's the real one. If the wrapper never reaches the rm -f (e.g. the user closes the terminal tab mid-run → SIGHUP, or the process gets a SIGTERM), the log stays behind in $TMPDIR.

It's 0600 from mktemp, same as the driver, so it's not readable by other users — it would just be one more copy of the password that nobody ever removes. The status file and the -c code file have the same gap today, so a single trap would cover all three. (I checked that bash runs an EXIT trap on both HUP and TERM.)

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done in eb047d1: one EXIT trap, armed before the first mktemp, now removes the status file, the topaz log and a -c run's code file. It replaces the rm -f lines at the end. A function rather than an array, because macOS's /bin/bash 3.2 treats an empty array as unbound under set -u. I measured that 3.2 runs the trap on TERM, HUP and INT, including when the whole process group gets the signal, and that the status passed to exit is unchanged.

One correction to the premise: the log doesn't hold the password. Topaz redacts it in the echo. The log line reads set user gemdb pass ******************************** (measured with the real 32-character password, and with a wrong one). The new refused-login test still checks that stderr carries nothing beyond GemStone's report. Either way the trap was worth doing, since the status and code files had the same gap.

New integration test: it starts gemdb forever.py in its own process group, sends SIGHUP to the group the way closing a terminal tab does, and requires $TMPDIR to be empty. It fails against the previous wrapper.

That test turned up one more thing: topaz survives the hangup and keeps running the program with its session. That was already true on main under -S, so it isn't from this PR. The test ends topaz with SIGTERM so the next test can stop the stone. On a real terminal the next write to the dead tty probably ends it, but a program that never prints would run on. I'll raise that separately.

Comment thread src/__integration__/cli.test.ts Outdated
Comment on lines +296 to +300
expect(ran.stderr).toContain('Login failed');
expect(ran.stderr).not.toContain('not-the-password');
expect(ran.stdout + ran.stderr).not.toContain('LEAKED');
expect(ran.stdout).not.toContain('hello from');
expect(ran.code).not.toBe(0);

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💭 Is it worth pinning what the user actually sees here, the way the Ctrl-C test now does with toEqual([...])?

As written, toContain('Login failed') plus the password check only cover the second sed expression (prompt lines). If the run … % range expression stopped matching — say topaz's prompt or echo format changes in the next alpha — the whole driver's Smalltalk would be dumped on stderr and this test would still pass. And since topaz "carries on through the script" after the refused login, whatever it says about the run block with no session also ends up here, which is exactly the kind of noise this PR is about.

Even a cheaper expect(ran.stderr).not.toContain('commandLineArguments') would catch the driver leaking. Similarly, could not.toBe(0) be the exact code the wrapper falls back to, so a change in that case branch is noticed?

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes. In eb047d1 the test pins stderr line by line to GemStone's report and nothing else:

-----------------------------------------------------
GemStone: Error         Fatal
Login failed:  the userId/password combination is invalid or expired.
Error Category: … Number: 4051 …
Arg 1: nil

So the driver leaking through, if the run … % range ever stops matching, fails it. So would anything topaz says about the run block it goes on through with no session; today it says nothing that reaches stderr (captured before pinning). Only the dashes and the 4051 line are matched loosely, so the category numbers can change in a future alpha.

The exit code is now toBe(1). That's topaz's own status, which the case passes on when there's no status file.

stdout is deliberately not pinned. The gem sometimes writes stone ibSocket read failed … SSL routines::wrong version number there after a failed login. That's the gem's own output, not topaz's, and it doesn't appear every time. stdout is still checked for the piped LEAKED line and for the program's output.

The status file, the topaz log and a -c run's code file were removed by
lines at the end of the wrapper, so a run whose terminal was closed, or
that was sent SIGTERM, left them in $TMPDIR. One EXIT trap, armed before
the first is made, now covers all three; bash runs it on SIGHUP and
SIGTERM as well (measured with macOS's /bin/bash 3.2).

The refused-login test now pins stderr to GemStone's report alone and
the exit status to 1, so the driver leaking through the log filter, or
topaz saying anything about the run block, fails it. A new test closes
a run's "terminal" mid-run and finds TMPDIR empty; against the previous
wrapper it fails. topaz itself outlives the hangup, as it did before, so
the test ends it.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@jgfoster
jgfoster added this pull request to the merge queue Oct 15, 2026
Merged via the queue into main with commit 6d78614 Oct 15, 2026
5 checks passed
@jgfoster
jgfoster deleted the james/quiet-ctrl-c branch October 15, 2026 21:07
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Ctrl+C in gemdb file mode prints ERROR 3004 and other topaz lines

2 participants