diff --git a/CHANGELOG.md b/CHANGELOG.md index 7165033c..24429932 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,5 +1,25 @@ # Changelog +## Unreleased + +### A terminal pane that loses its socket now reconnects when you touch it + +- **Typing into a dead pane was a silent drain.** `term.onData` guarded on + `readyState === 1` and did nothing else, so every keystroke vanished with no + reconnect and no sign anything was wrong. xterm renders locally, so the pane looked + alive — that is the "terminal freezes until I reload the browser" report. +- **Paste and the send line named the problem and did nothing about it.** Both toasted + "Terminal is not connected" and returned, on the two controls that exist precisely + because a pane has gone unresponsive. All three paths now revive the socket and + carry the frame through, so the first thing you type is not the thing that is lost. +- **A terminal socket failure used to leave no trace.** `ws.onclose` took no argument + (the close code was discarded) and there was no `onerror` at all. Both exist now, so + the next occurrence says whether it was an abnormal close, an oversized frame or a + server error instead of looking identical to every other cause. + +Note: this fixes what the UI does about a dropped terminal socket. What drops it in +the first place is not yet identified — the close code added here is what will name it. + ## 7.9.0 A remote chat can no longer get stuck for good, a broken local Claude CLI install diff --git a/CLAUDE.md b/CLAUDE.md index a2e410df..4112f605 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -442,6 +442,22 @@ Four reports that all reduce to "the UI moved or stopped listening". Pinned by panels animate `width .25s`, so measuring from the click measures the box the user is leaving. The observer compares `clientWidth` before re-measuring — the callback sets the height, so an unguarded one re-enters forever. +- **Touching a pane is itself a reconnect trigger (`_reviveTerminal`).** Every + automatic path is conditional on a state a half-dead socket does not report, so all + three interaction paths used to give up instead: `term.onData` dropped the keystroke + SILENTLY (the pane renders locally, so it looks alive — this is the "terminal froze + until I reloaded the page" report), while paste and the send line toasted "not + connected" and returned, naming the problem on the very controls that exist because + a pane went unresponsive. They now revive and carry the frame. Only the LAST frame + is queued — a queue of stale keystrokes replayed into a TUI is worse than a dropped + one — a socket already CONNECTING is not raced with a second one, and `_pending` is + cleared BEFORE the send so a failure cannot replay it on the next open too. +- **A close code is the only evidence of WHY a pane died.** `ws.onclose` took no + argument and there was no `onerror` at all, so a terminal socket failure left no + trace on either end — which is why "it freezes sometimes" stayed unexplained. Both + exist now; the log sits AFTER the staleness guard, because a superseded socket + closing is routine and would bury the closes that mean something. 1006 is an + abnormal close with no FIN, 1009 a frame over `maxPayload`, 1011 a server error. - **`sendToTerminal()` has no emptiness guard, on purpose.** A bare Return is the most useful thing this line can send: it accepts a prompt's default (`[Y/n]`, "press enter to continue"). `showTerminalView()` clears the box, because one send line serves every diff --git a/public/index.html b/public/index.html index 429c29b7..074c1dc3 100644 --- a/public/index.html +++ b/public/index.html @@ -17793,7 +17793,16 @@

⌨️ Keyboard Shortcuts

// Bound to `entry.ws`, not a captured socket — so a later reconnect (fresh // WebSocket swapped into entry.ws) keeps routing keystrokes without re-wiring. - term.onData(d => { if (entry.ws && entry.ws.readyState === 1) entry.ws.send(JSON.stringify({ type: 'input', data: d })); }); + // A dead socket used to make this a silent drain: the guard was `readyState === 1` + // and nothing else, so every keystroke vanished with no reconnect and no sign that + // anything was wrong. That IS the "terminal froze until I reloaded the page" report + // — the pane looks live because xterm renders locally. + term.onData(d => { + if (entry.ws && entry.ws.readyState === 1) { entry.ws.send(JSON.stringify({ type: 'input', data: d })); return; } + // Typing is the clearest possible "I am using this pane". Reconnect on it, and + // queue the keystroke so the first thing typed is not the thing that gets lost. + _reviveTerminal(sessionId, { type: 'input', data: d }); + }); // Anything that steals focus — a modal, a toast button, the send line below, the // browser restoring focus after an alt-tab — leaves the pane visually active but // deaf. A click on it is an unambiguous "I am typing here"; don't steal focus back @@ -17812,6 +17821,42 @@

⌨️ Keyboard Shortcuts

// scrollback are left untouched; only the socket is replaced, and the server // re-paints the current tmux pane on attach (see terminal-bridge.js captureScreen), // so the view catches back up to what actually happened while disconnected. +// Bring a pane back the moment the user touches it, and don't lose what they were +// trying to send. Every automatic reconnect path is conditional on a state a +// half-dead socket does not report: onclose only runs if a close event arrives, +// visibilitychange only fires on a tab switch, and the backoff can be 30s out. +// Meanwhile the three interaction paths (typing, paste, the send line) each just +// gave up — one silently, two with a toast that named the problem and did nothing +// about it. This is the "just reconnect and carry on" the pane always implied. +// `frame` is optional and replayed once the socket opens; only the LAST one is +// kept, because a queue of stale keystrokes replayed into a TUI is worse than a +// dropped one. `exited` is cleared for the same reason refreshTerminalSession() +// clears it: a latched pane never retries, and the user asking for it outranks the +// latch. Returns true when the socket was already live and nothing was queued. +function _reviveTerminal(sessionId, frame) { + const entry = _terms.get(sessionId); if (!entry) return false; + if (entry.ws && entry.ws.readyState === 1) { + if (frame) { try { entry.ws.send(JSON.stringify(frame)); } catch {} } + return true; + } + if (frame) entry._pending = frame; + // A socket still CONNECTING will deliver _pending from its own onopen; opening a + // second one on every keystroke would stampede the server. + if (entry.ws && entry.ws.readyState === 0) return false; + entry.exited = false; + entry._reconnectDelay = 1000; + _connectTerminalWs(sessionId); + return false; +} + +// Deliver what the user typed or pasted into a socket that turned out to be dead. +// Cleared BEFORE the send, so a failing send cannot replay it on the next open too. +function _flushTerminalPending(entry, ws) { + if (!entry || !entry._pending) return; + const f = entry._pending; entry._pending = null; + try { ws.send(JSON.stringify(f)); } catch {} +} + function _connectTerminalWs(sessionId) { const entry = _terms.get(sessionId); if (!entry) return; const term = entry.term; @@ -17830,6 +17875,7 @@

⌨️ Keyboard Shortcuts

ws.binaryType = 'arraybuffer'; ws.onopen = () => { entry._reconnectDelay = 1000; + _flushTerminalPending(entry, ws); if (_termSessionId !== sessionId) return; sendTermResize(); // xterm routes keystrokes through a hidden textarea. A reconnect swaps entry.ws @@ -17886,12 +17932,23 @@

⌨️ Keyboard Shortcuts

} _paintTermStatus(sessionId); }; - ws.onclose = () => { + // The chat socket has had one of these since it was written (search `onerror`); + // the terminal never did, so a socket-level failure left no trace anywhere — + // which is why "the terminal freezes sometimes" stayed unexplained. + ws.onerror = () => { console.warn('[terminal] socket error', { session: sessionId }); }; + ws.onclose = (ev) => { // Staleness first, and it guards the STATUS too, not just the retry. This socket // may already have been superseded (refreshTerminalSession() detaches before it // closes) -- painting "disconnected" from here would mark the pane the user is // typing into as dead while it is working. if (_terms.get(sessionId) !== entry || entry.ws !== ws) return; + // `ev` used to be ignored entirely, and there was no onerror either, so a pane + // that died left no trace anywhere — which is why "the terminal freezes + // sometimes" stayed unexplained. A close code is the only evidence of WHY: + // 1006 abnormal/no FIN, 1001 going away, 1009 frame too big, 1011 server error. + // Logged AFTER the staleness guard on purpose: a superseded socket closing is + // routine, and logging it would bury the real ones. + if (ev && ev.code !== 1000) console.warn('[terminal] closed', { session: sessionId, code: ev.code, reason: ev.reason || '' }); entry.dot = '#8b949e'; entry.state = t('term.state.disconnected'); _paintTermStatus(sessionId); // Auto-retry an unexpected drop (network blip, server restart, idle proxy // timeout) even while this terminal tab stays in the foreground the whole @@ -17943,7 +18000,14 @@

⌨️ Keyboard Shortcuts

// enter to continue"), which is exactly what this line exists to reach. const text = box.value; const entry = _termSessionId && _terms.get(_termSessionId); - if (!entry || !entry.ws || entry.ws.readyState !== 1) { toast(t('term.paste.notconnected'), true); return; } + // Reconnect and deliver, rather than refusing: this line is most useful exactly + // when a pane has gone unresponsive, which is also when the old guard refused. + if (!entry || !entry.ws || entry.ws.readyState !== 1) { + _reviveTerminal(_termSessionId, { type: 'input', data: text + '\r' }); + box.value = ''; + toast(t('term.state.reconnecting')); + return; + } // \r, not \n: a PTY sees the Return KEY, and a TUI reading in canonical or raw mode // both treat CR as submit. Sent as ONE frame with the text so a prompt that echoes // and submits on the same tick cannot split them. @@ -18010,7 +18074,12 @@

⌨️ Keyboard Shortcuts

const sid = _terminalPasteTarget(); if (!sid) { toast(t('term.paste.notarget'), true); return; } const entry = _terms.get(sid); - if (!entry?.ws || entry.ws.readyState !== 1) { toast(t('term.paste.notconnected'), true); return; } + if (!entry?.ws || entry.ws.readyState !== 1) { + _reviveTerminal(sid, { type: 'paste', data: text }); + switchTab(sid); + toast(t('term.state.reconnecting')); + return; + } entry.ws.send(JSON.stringify({ type: 'paste', data: text })); switchTab(sid); toast(t('term.paste.done')); diff --git a/test/composer-terminal.test.js b/test/composer-terminal.test.js index 5af7858e..43057537 100644 --- a/test/composer-terminal.test.js +++ b/test/composer-terminal.test.js @@ -120,12 +120,24 @@ check('it reconnects unconditionally — no readyState test', /readyState/.test( // A close event from a socket refreshTerminalSession() already replaced must not // repaint the pane the user is typing into. The guard has to sit ABOVE the paint, // not only above the retry — nothing repaints the status back afterwards. -const onclose = TB.slice(TB.indexOf('ws.onclose = () => {')); +// The handler takes the close EVENT: a close code is the only evidence of why a pane +// died (1006 abnormal, 1009 frame too big, 1011 server error), and it used to be +// discarded, which is why "the terminal freezes sometimes" had no trace to follow. +check('the close handler receives the event', TB.includes('ws.onclose = (ev) => {'), true); +const onclose = TB.slice(TB.indexOf('ws.onclose = (ev) => {')); const oncloseBody = onclose.slice(0, onclose.indexOf('\n };') + 5); // indexOf alone would pass on -1 (guard deleted) < paint index — assert presence first. check('a stale close is ignored, and before it touches the status', oncloseBody.includes('entry.ws !== ws) return;') && oncloseBody.indexOf('entry.ws !== ws) return;') < oncloseBody.indexOf("term.state.disconnected"), true); +// …and before the logging too: a superseded socket closing is routine, and logging +// it would bury the closes that actually explain something. +check('the close code is logged, after the staleness guard', + oncloseBody.includes('ev.code') && + oncloseBody.indexOf('entry.ws !== ws) return;') < oncloseBody.indexOf('ev.code'), true); +// The chat socket has had one since it was written; the terminal had none, so a +// socket-level failure was invisible on both ends. +check('the terminal socket has an onerror at all', /ws\.onerror = \(\) =>/.test(TB), true); console.log('\n— 4. sending a line into a terminal session —'); @@ -145,8 +157,15 @@ const stsBody = sts.slice(0, sts.indexOf('\n}\n') + 3); // on the engine pane alike, where a `view=engine` viewer may only send `input`. check('it writes the terminal protocol\'s input frame', /type: 'input', data: text \+ '\\r'/.test(stsBody), true); check('CR, not LF — a PTY needs the Return key', /'\\n'/.test(stsBody), false); -check('it refuses to send on a socket that is not OPEN', /readyState !== 1/.test(stsBody), true); -check('and says so instead of failing silently', /toast\(t\('term\.paste\.notconnected'\), true\)/.test(stsBody), true); +check('it notices a socket that is not OPEN', /readyState !== 1/.test(stsBody), true); +// It used to toast "not connected" and return — naming the problem and doing nothing +// about it, on the one control that exists BECAUSE a pane has gone unresponsive. +// Every automatic reconnect path is conditional on a state a half-dead socket does +// not report, so touching the pane has to be a reconnect trigger of its own. +check('and reconnects instead of refusing', /_reviveTerminal\(/.test(stsBody), true); +check('carrying the line, so the first thing typed is not the thing that is lost', + /_reviveTerminal\([^)]*,\s*\{ type: 'input', data: text \+ '\\r' \}\)/.test(stsBody), true); +check('the dead-socket refusal is gone', /paste\.notconnected/.test(stsBody), false); check('it clears the box after sending', /box\.value = ''/.test(stsBody), true); // A bare Return is the single most useful thing to send: it accepts a prompt default. check('an empty line is still sendable', /!text\.trim\(\)\) return/.test(stsBody), false); @@ -154,6 +173,30 @@ check('an empty line is still sendable', /!text\.trim\(\)\) return/.test(stsBody check('switching panes clears the shared send line', /function showTerminalView[\s\S]{0,1600}?\$i\('termSendInput'\)[^\n]*value = ''/.test(TB), true); +console.log('\n— a dead socket must not swallow what the user does —'); +// The silent drain that produced "the terminal froze until I reloaded the page": +// the pane renders locally, so it looks alive while every keystroke goes nowhere. +{ + const od = TB.slice(TB.indexOf('term.onData(d => {')); + const odBody = od.slice(0, od.indexOf('\n });') + 6); + check('onData still fast-paths a live socket', /readyState === 1/.test(odBody), true); + check('and revives a dead one instead of dropping the keystroke', + /_reviveTerminal\(sessionId, \{ type: 'input', data: d \}\)/.test(odBody), true); + // Reconnecting on every keystroke would stampede the server; a socket already + // CONNECTING will deliver the queued frame from its own onopen. + const rv = TB.slice(TB.indexOf('function _reviveTerminal')); + const rvBody = rv.slice(0, rv.indexOf('\n}\n') + 3); + check('a CONNECTING socket is not raced with a second one', /readyState === 0\) return false/.test(rvBody), true); + check('only the LAST frame is queued — stale keystrokes replayed into a TUI are worse', + /entry\._pending = frame/.test(rvBody), true); + check('it clears the exited latch, like the manual refresh does', /entry\.exited = false/.test(rvBody), true); + // Cleared before the send, or a failing send replays it on the next open as well. + const fl = TB.slice(TB.indexOf('function _flushTerminalPending')); + const flBody = fl.slice(0, fl.indexOf('\n}\n') + 3); + check('the queued frame is cleared BEFORE it is sent', + flBody.indexOf('entry._pending = null') < flBody.indexOf('ws.send'), true); +} + console.log('\n— focus: why a deaf pane used to need a tab switch —'); check('a reconnect refocuses the pane',