terminal: stop replaying questions, stop opening two sockets, bind the word keys
three separate faults behind "reconnecting gets weird and the keyboard is not natural". ── the replay typed into the shell ── the pty buffer was stored raw and replayed verbatim on every re-attach. anything in it that ASKS the terminal a question — DSR, DA, DECRQM, XTVERSION, XTGETTCAP, the OSC colour queries — got asked again, and xterm answered correctly by writing the reply to its input. the pty receives that as a keystroke nobody typed. stripped on the way IN, since the buffer is the thing that gets replayed and a live client already answered them once when they were legitimately asked. only questions are removed; everything that draws is untouched. where a control shares its final byte with one that draws, the parameter is enumerated rather than wildcarded — CSI 18 t asks the window size, CSI 22 t pushes the title, and stripping the second would change what a replay renders. 36 tests, both directions, because both fail silently. ── two sockets on one session ── handleClose armed a reconnect timer; handleVisibilityChange fired on tab focus whenever readyState was CLOSED — which is exactly what a pending timer leaves. both ran. every keystroke went twice, two replay frames fought over the screen, and only one socket was ever cleaned up because __terminalCleanup is overwritten by whichever connect ran last. connect() is now the single guard, and a stale socket's close no longer speaks for the session. ── the keyboard ── alt-arrow was dead for everyone: xterm.js 5 rewrote it into the ctrl-arrow sequence, xterm.js 6 removed that rewrite and emits the honest ^[[1;3C/D (verified — the string 1;3D does not appear anywhere in the 6.0 bundle). nothing bound it. so it broke on a dependency bump, with no shell config changed. bound in zsh rather than translated in the browser, deliberately: tmux.conf claims M-Left/M-Right for pane switching, and a client-side rewrite would send ^[b to tmux and break it. the real sequence lets tmux handle it inside a session and zsh outside. ctrl-arrow was worse and more embarrassing: it worked for MEMBERS and not for the OWNER. shell-skel/zshrc has had the bindings all along; the owner's .zshrc is assembled in machine-setup and never got them. the owner had a strictly worse shell than the accounts they provision. confirmed with `zsh -i -c bindkey` before and after. also: escape-time 10 in tmux.conf. the 500ms default delays every Alt chord and every Escape, which is most of what "not natural" felt like. applied to this host by hand — setup only runs at install. cmd+arrow is left alone: xterm emits nothing for it, so there is no sequence to bind. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
This commit is contained in:
@@ -2311,6 +2311,41 @@ EOF
|
|||||||
ok "~/.local/bin and ~/.opencode/bin added to PATH"
|
ok "~/.local/bin and ~/.opencode/bin added to PATH"
|
||||||
fi
|
fi
|
||||||
|
|
||||||
|
# ── Keys ──
|
||||||
|
#
|
||||||
|
# This block existed only in `src/servers/shell-skel/zshrc`, the file the platform seeds into
|
||||||
|
# PROVISIONED MEMBER accounts. The owner's .zshrc is assembled here instead, and never got it — so
|
||||||
|
# the owner had a strictly worse shell than the members they provision: no ctrl-arrow, no
|
||||||
|
# history-prefix search, no Home/End. Confirmed on this machine before writing it, with
|
||||||
|
# `zsh -i -c bindkey`: the owner had `^[b`/`^[f` and nothing else.
|
||||||
|
#
|
||||||
|
# The two files are still separate — one is a template the platform copies, the other is an
|
||||||
|
# idempotent append — but the KEYS have to agree, because a member and the owner sit at the same
|
||||||
|
# web terminal and neither should have to learn which account they are on.
|
||||||
|
if append_once "$ZSHRC" keybindings <<'EOF'
|
||||||
|
bindkey -e
|
||||||
|
autoload -Uz up-line-or-beginning-search down-line-or-beginning-search
|
||||||
|
zle -N up-line-or-beginning-search
|
||||||
|
zle -N down-line-or-beginning-search
|
||||||
|
bindkey '^[[A' up-line-or-beginning-search
|
||||||
|
bindkey '^[[B' down-line-or-beginning-search
|
||||||
|
bindkey '^[[1;5C' forward-word
|
||||||
|
bindkey '^[[1;5D' backward-word
|
||||||
|
bindkey '^[[1;3C' forward-word
|
||||||
|
bindkey '^[[1;3D' backward-word
|
||||||
|
bindkey '^[[3~' delete-char
|
||||||
|
bindkey '^[[H' beginning-of-line
|
||||||
|
bindkey '^[[F' end-of-line
|
||||||
|
bindkey '^[[1~' beginning-of-line
|
||||||
|
bindkey '^[[4~' end-of-line
|
||||||
|
bindkey '^H' backward-kill-word
|
||||||
|
bindkey '^[^?' backward-kill-word
|
||||||
|
bindkey '^[[3;5~' kill-word
|
||||||
|
EOF
|
||||||
|
then
|
||||||
|
ok "shell keybindings added (ctrl/alt-arrow, history search, Home/End)"
|
||||||
|
fi
|
||||||
|
|
||||||
# The eza aliases are GUARDED and the rest are not, for one reason: these
|
# The eza aliases are GUARDED and the rest are not, for one reason: these
|
||||||
# replace `ls`. An unguarded `alias ls='eza --icons'` on a machine where eza
|
# replace `ls`. An unguarded `alias ls='eza --icons'` on a machine where eza
|
||||||
# failed to install leaves the owner with no working `ls` at all, in every new
|
# failed to install leaves the owner with no working `ls` at all, in every new
|
||||||
|
|||||||
@@ -24,6 +24,12 @@ bind - split-window -v
|
|||||||
unbind r
|
unbind r
|
||||||
bind r source-file ~/.tmux.conf \; display-message "Config reloaded!" \; refresh-client -S
|
bind r source-file ~/.tmux.conf \; display-message "Config reloaded!" \; refresh-client -S
|
||||||
|
|
||||||
|
# Meta keys are ESC-prefixed on the wire, and tmux waits `escape-time` to decide whether an incoming ESC is
|
||||||
|
# a lone Escape or the start of one. The default is 500ms, so every Alt-chord below — and every Escape in
|
||||||
|
# vim — pays half a second before anything happens. 10ms is enough to disambiguate a sequence that arrives
|
||||||
|
# in one TCP frame, which over a websocket relay it always does.
|
||||||
|
set -sg escape-time 10
|
||||||
|
|
||||||
# switch panes using Alt-arrow without prefix
|
# switch panes using Alt-arrow without prefix
|
||||||
bind -n M-Left select-pane -L
|
bind -n M-Left select-pane -L
|
||||||
bind -n M-Right select-pane -R
|
bind -n M-Right select-pane -R
|
||||||
|
|||||||
@@ -58,9 +58,30 @@ bindkey '^[[A' up-line-or-beginning-search # Up: history matching what is alr
|
|||||||
bindkey '^[[B' down-line-or-beginning-search
|
bindkey '^[[B' down-line-or-beginning-search
|
||||||
bindkey '^[[1;5C' forward-word # ctrl-arrow by word
|
bindkey '^[[1;5C' forward-word # ctrl-arrow by word
|
||||||
bindkey '^[[1;5D' backward-word
|
bindkey '^[[1;5D' backward-word
|
||||||
|
bindkey '^[[1;3C' forward-word # alt-arrow by word — see below
|
||||||
|
bindkey '^[[1;3D' backward-word
|
||||||
bindkey '^[[3~' delete-char
|
bindkey '^[[3~' delete-char
|
||||||
bindkey '^[[H' beginning-of-line
|
bindkey '^[[H' beginning-of-line
|
||||||
bindkey '^[[F' end-of-line
|
bindkey '^[[F' end-of-line
|
||||||
|
bindkey '^[[1~' beginning-of-line # the other Home/End encoding; terminals disagree
|
||||||
|
bindkey '^[[4~' end-of-line
|
||||||
|
bindkey '^H' backward-kill-word # ctrl-backspace (and alt-backspace, which sends ^[^?)
|
||||||
|
bindkey '^[^?' backward-kill-word
|
||||||
|
bindkey '^[[3;5~' kill-word # ctrl-delete
|
||||||
|
|
||||||
|
# Why alt-arrow needs binding at all, given ^[b/^[f already work.
|
||||||
|
#
|
||||||
|
# xterm.js 5 rewrote Alt+Left/Right into the ctrl-arrow sequence, so `^[[1;5D` covered both. **xterm.js 6
|
||||||
|
# removed that rewrite** (verified: the string `1;3D` does not appear anywhere in the 6.0 bundle), and now
|
||||||
|
# emits the honest `^[[1;3D`. Nothing bound it, so alt-arrow became a no-op the moment the dependency moved
|
||||||
|
# — on a machine where nobody had changed a line of shell config.
|
||||||
|
#
|
||||||
|
# Bound here rather than translated in the browser on purpose: `tmux.conf` claims M-Left/M-Right for pane
|
||||||
|
# switching, and a client-side rewrite would send `^[b` to tmux and break that. Letting the real sequence
|
||||||
|
# through means tmux gets it inside a session and zsh gets it outside, which is what both expect.
|
||||||
|
#
|
||||||
|
# Cmd+Left/Right is deliberately absent: xterm emits NOTHING for it (`case 37: if (e.metaKey) break`), so
|
||||||
|
# there is no sequence to bind. On macOS that chord reaches the browser as back/forward.
|
||||||
|
|
||||||
# ── Editor ──
|
# ── Editor ──
|
||||||
if command -v nvim >/dev/null 2>&1; then
|
if command -v nvim >/dev/null 2>&1; then
|
||||||
|
|||||||
@@ -46,8 +46,40 @@ const resolveCwd = (cwd, base) => {
|
|||||||
return home;
|
return home;
|
||||||
};
|
};
|
||||||
|
|
||||||
|
/**
|
||||||
|
* Sequences that make a terminal ANSWER, stripped before anything is stored.
|
||||||
|
*
|
||||||
|
* The scrollback is replayed verbatim to a re-attaching client. Anything in it that asks the terminal a
|
||||||
|
* question gets asked AGAIN on every reconnect — and xterm answers, correctly, by writing the reply to its
|
||||||
|
* input. That input is a keystroke as far as the pty is concerned, so a reconnect injects text into the
|
||||||
|
* shell that nobody typed: `^[[?62;c` and friends landing on the command line, or being eaten by whatever
|
||||||
|
* TUI is running. It is the "terminal goes weird after reconnecting" symptom, and it is not the shell's
|
||||||
|
* fault.
|
||||||
|
*
|
||||||
|
* Stripping on the way IN rather than on the way out: the buffer is the thing that gets replayed, and a
|
||||||
|
* live client has already answered these once, at the moment they were legitimately asked.
|
||||||
|
*
|
||||||
|
* What is removed is only ever a QUESTION. Colour, cursor movement, screen clears — everything that draws —
|
||||||
|
* is untouched, so a replay still reproduces the screen exactly.
|
||||||
|
*/
|
||||||
|
// Each pattern is the QUERY form only. Where a control shares its final byte with a command that DRAWS,
|
||||||
|
// the numeric parameter is enumerated rather than wildcarded — `CSI 18 t` asks the window size, but
|
||||||
|
// `CSI 22 t` pushes the title, and stripping the second would silently change what a replay renders.
|
||||||
|
const QUERY_SEQUENCES = [
|
||||||
|
/\x1b\[\??[56]n/g, // DSR — cursor position (6n), status (5n), and the DEC `?` variants
|
||||||
|
/\x1b\[[0-9;?>=]*c/g, // DA1/DA2/DA3 — device attributes. `c` is only ever a query.
|
||||||
|
/\x1b\[\?[0-9;]*\$p/g, // DECRQM — mode query
|
||||||
|
/\x1b\[(?:1[1345689]|2[01])(?:;[0-9]+)*t/g, // XTWINOPS reports only — NOT 22/23 (title push/pop)
|
||||||
|
/\x1b\[>[0-9;]*q/g, // XTVERSION
|
||||||
|
/\x1bP\+q[0-9a-fA-F;]*(?:\x1b\\|\x07)/g, // DCS XTGETTCAP — terminfo capability query
|
||||||
|
/\x1b\](?:10|11|12|4;[0-9]+);\?(?:\x07|\x1b\\)/g, // OSC colour queries (fg/bg/cursor/palette)
|
||||||
|
];
|
||||||
|
|
||||||
|
/** Exported for the test: this is the one function here whose mistakes are invisible until a replay. */
|
||||||
|
export const stripQueries = (data) => QUERY_SEQUENCES.reduce((out, re) => out.replace(re, ''), data);
|
||||||
|
|
||||||
const appendBuffer = (session, data) => {
|
const appendBuffer = (session, data) => {
|
||||||
session.buffer += data;
|
session.buffer += stripQueries(data);
|
||||||
if (session.buffer.length > BUFFER_MAX) {
|
if (session.buffer.length > BUFFER_MAX) {
|
||||||
// Cut on a line boundary, not a byte offset. A blind slice can land inside an escape sequence, and the
|
// Cut on a line boundary, not a byte offset. A blind slice can land inside an escape sequence, and the
|
||||||
// replay then opens with the tail of a colour or cursor-move code — which xterm renders as garbage, or
|
// replay then opens with the tail of a colour or cursor-move code — which xterm renders as garbage, or
|
||||||
|
|||||||
@@ -0,0 +1,82 @@
|
|||||||
|
import { describe, test, expect } from 'bun:test';
|
||||||
|
// The sidecar is .mjs — node-pty binds a native addon against node's ABI, so this half of the tree is plain
|
||||||
|
// JavaScript. Only the pure function is imported; nothing here spawns a pty.
|
||||||
|
import { stripQueries } from './sessions.mjs';
|
||||||
|
|
||||||
|
// The scrollback is replayed verbatim when a client re-attaches. Anything in it that ASKS the terminal a
|
||||||
|
// question is asked again on every reconnect, and xterm answers by writing the reply to its input — which
|
||||||
|
// the pty receives as a keystroke nobody typed. That is the whole reason this function exists.
|
||||||
|
//
|
||||||
|
// It is tested rather than eyeballed because both directions fail silently: strip too little and a reconnect
|
||||||
|
// injects junk into the shell, strip too much and a replay renders differently from the live screen, and
|
||||||
|
// neither shows up until someone reconnects at the wrong moment.
|
||||||
|
|
||||||
|
describe('stripQueries — removes what would be answered', () => {
|
||||||
|
const cases: Array<[string, string]> = [
|
||||||
|
['cursor position report (DSR 6n)', '\x1b[6n'],
|
||||||
|
['status report (DSR 5n)', '\x1b[5n'],
|
||||||
|
['DEC variant (DSR ?6n)', '\x1b[?6n'],
|
||||||
|
['primary device attributes', '\x1b[c'],
|
||||||
|
['secondary device attributes', '\x1b[>c'],
|
||||||
|
['tertiary device attributes', '\x1b[=c'],
|
||||||
|
['DA with parameters', '\x1b[0c'],
|
||||||
|
['mode query (DECRQM)', '\x1b[?2026$p'],
|
||||||
|
['window size report (XTWINOPS 18)', '\x1b[18t'],
|
||||||
|
['text area size (XTWINOPS 14)', '\x1b[14t'],
|
||||||
|
['version query (XTVERSION)', '\x1b[>0q'],
|
||||||
|
['terminfo capability query (XTGETTCAP)', '\x1bP+q544e\x1b\\'],
|
||||||
|
['foreground colour query', '\x1b]10;?\x07'],
|
||||||
|
['background colour query', '\x1b]11;?\x1b\\'],
|
||||||
|
['cursor colour query', '\x1b]12;?\x07'],
|
||||||
|
['palette entry query', '\x1b]4;1;?\x07'],
|
||||||
|
];
|
||||||
|
|
||||||
|
for (const [name, sequence] of cases) {
|
||||||
|
test(`strips ${name}`, () => {
|
||||||
|
expect(stripQueries(`before${sequence}after`)).toBe('beforeafter');
|
||||||
|
});
|
||||||
|
}
|
||||||
|
|
||||||
|
test('strips several in one chunk, and repeats of the same one', () => {
|
||||||
|
expect(stripQueries('a\x1b[6nb\x1b[cc\x1b[6nd')).toBe('abcd');
|
||||||
|
});
|
||||||
|
});
|
||||||
|
|
||||||
|
describe('stripQueries — keeps everything that draws', () => {
|
||||||
|
const kept: Array<[string, string]> = [
|
||||||
|
['SGR colour', '\x1b[31mred\x1b[0m'],
|
||||||
|
['256-colour SGR', '\x1b[38;5;208m'],
|
||||||
|
['truecolour SGR', '\x1b[38;2;255;128;0m'],
|
||||||
|
['cursor movement', '\x1b[10;20H'],
|
||||||
|
['cursor up', '\x1b[3A'],
|
||||||
|
['erase in display', '\x1b[2J'],
|
||||||
|
['erase in line', '\x1b[K'],
|
||||||
|
['alt screen on', '\x1b[?1049h'],
|
||||||
|
['alt screen off', '\x1b[?1049l'],
|
||||||
|
['bracketed paste on', '\x1b[?2004h'],
|
||||||
|
['mouse tracking on', '\x1b[?1000h'],
|
||||||
|
['scroll region', '\x1b[1;24r'],
|
||||||
|
['window title (OSC 0)', '\x1b]0;my title\x07'],
|
||||||
|
['window title (OSC 2)', '\x1b]2;my title\x07'],
|
||||||
|
['OSC 52 clipboard write', '\x1b]52;c;aGVsbG8=\x07'],
|
||||||
|
['save cursor', '\x1b7'],
|
||||||
|
['plain text with newlines', 'line one\r\nline two\r\n'],
|
||||||
|
];
|
||||||
|
|
||||||
|
for (const [name, sequence] of kept) {
|
||||||
|
test(`keeps ${name}`, () => {
|
||||||
|
expect(stripQueries(sequence)).toBe(sequence);
|
||||||
|
});
|
||||||
|
}
|
||||||
|
|
||||||
|
// The pair that made the XTWINOPS pattern enumerate its parameters instead of wildcarding them: both are
|
||||||
|
// `CSI … t`, one is a question and the other changes what a replay renders.
|
||||||
|
test('keeps title push/pop (XTWINOPS 22/23) while stripping the reports', () => {
|
||||||
|
expect(stripQueries('\x1b[22;0t\x1b[18t\x1b[23;0t')).toBe('\x1b[22;0t\x1b[23;0t');
|
||||||
|
});
|
||||||
|
|
||||||
|
test('leaves a realistic prompt untouched', () => {
|
||||||
|
const prompt = '\x1b]0;green@edge: ~\x07\x1b[1;32mgreen@edge\x1b[0m:\x1b[1;34m~\x1b[0m$ ';
|
||||||
|
expect(stripQueries(prompt)).toBe(prompt);
|
||||||
|
});
|
||||||
|
});
|
||||||
@@ -243,6 +243,26 @@ export const TerminalView = ({
|
|||||||
const connect = () => {
|
const connect = () => {
|
||||||
if (disposed) return;
|
if (disposed) return;
|
||||||
|
|
||||||
|
// ── One socket per session, enforced here rather than at each caller ──
|
||||||
|
//
|
||||||
|
// There are two independent reconnect triggers — `handleClose` arms a timer, and
|
||||||
|
// `handleVisibilityChange` fires when the tab comes back — and they overlap exactly: a pending timer
|
||||||
|
// leaves `readyState === CLOSED`, which is the visibility handler's own condition to reconnect. Both
|
||||||
|
// then ran, and the session ended up with TWO live sockets: every keystroke delivered twice, two
|
||||||
|
// `replay` frames fighting over the screen, two `resize` frames racing. That is the "goes weird after
|
||||||
|
// reconnecting" report, and it is also why only one socket was ever cleaned up —
|
||||||
|
// `__terminalCleanup` is overwritten by whichever connect ran last.
|
||||||
|
//
|
||||||
|
// Guarding the entry point covers both callers and any future third one.
|
||||||
|
if (reconnectTimer) {
|
||||||
|
clearTimeout(reconnectTimer);
|
||||||
|
reconnectTimer = null;
|
||||||
|
}
|
||||||
|
const existing = wsRef.current;
|
||||||
|
if (existing && (existing.readyState === WebSocket.OPEN || existing.readyState === WebSocket.CONNECTING)) {
|
||||||
|
return;
|
||||||
|
}
|
||||||
|
|
||||||
fitAddon.fit();
|
fitAddon.fit();
|
||||||
const cols = term.cols;
|
const cols = term.cols;
|
||||||
const rows = term.rows;
|
const rows = term.rows;
|
||||||
@@ -335,6 +355,10 @@ export const TerminalView = ({
|
|||||||
|
|
||||||
const handleClose = () => {
|
const handleClose = () => {
|
||||||
cleanupWs();
|
cleanupWs();
|
||||||
|
// A socket that is no longer the session's must not arm a reconnect: its close arrives AFTER the
|
||||||
|
// replacement is already open, and the timer it sets would then tear down a healthy connection to
|
||||||
|
// build a third. Only the current socket speaks for the session.
|
||||||
|
if (wsRef.current !== ws) return;
|
||||||
if (disposed || processExited) {
|
if (disposed || processExited) {
|
||||||
onConnectionChangeRef.current?.('disconnected');
|
onConnectionChangeRef.current?.('disconnected');
|
||||||
term.write('\r\n[Disconnected]\r\n');
|
term.write('\r\n[Disconnected]\r\n');
|
||||||
@@ -381,13 +405,13 @@ export const TerminalView = ({
|
|||||||
});
|
});
|
||||||
|
|
||||||
// Reconnect when tab becomes visible again
|
// Reconnect when tab becomes visible again
|
||||||
|
// Coming back to the tab should not WAIT out a backoff that may be seconds long — but it must not open a
|
||||||
|
// second socket either. `connect` now decides that (it returns early on a live one), so this only has to
|
||||||
|
// say "try now", and the CLOSED/CLOSING distinction that used to be here stops mattering.
|
||||||
const handleVisibilityChange = () => {
|
const handleVisibilityChange = () => {
|
||||||
if (document.visibilityState === 'visible' && !disposed && !processExited) {
|
if (document.visibilityState !== 'visible' || disposed || processExited) return;
|
||||||
if (!wsRef.current || wsRef.current.readyState === WebSocket.CLOSED) {
|
|
||||||
reconnectAttempts = 0;
|
reconnectAttempts = 0;
|
||||||
connect();
|
connect();
|
||||||
}
|
|
||||||
}
|
|
||||||
};
|
};
|
||||||
document.addEventListener('visibilitychange', handleVisibilityChange);
|
document.addEventListener('visibilitychange', handleVisibilityChange);
|
||||||
|
|
||||||
|
|||||||
Reference in New Issue
Block a user