diff --git a/docs/pr18-fix-plan.md b/docs/pr18-fix-plan.md new file mode 100644 index 0000000..6edfa00 --- /dev/null +++ b/docs/pr18-fix-plan.md @@ -0,0 +1,32 @@ +# PR18 terminal fix plan + +## Problem +PR #18 fixed terminal corruption and response-garbage issues, but the merged frontend implementation still has two material defects in `internal/ui/static/index.html`: + +1. The WebSocket binary path decodes each frame independently with `TextDecoder().decode(u8)`, which can corrupt multibyte UTF-8 characters when a character is split across frames. +2. The SSE fallback path writes `msg.chunk` directly to the terminal and bypasses the new response-filtering logic, so the same garbage strings can still leak through when WebSocket is unavailable or times out. + +## Current state analysis +- Output handling is concentrated in `internal/ui/static/index.html`. +- Query-response filtering is currently duplicated across `loadLog()`, `loadLogSince()`, WebSocket text handling, and WebSocket binary handling. +- `term.onData()` already has a separate input-side guard via `isTerminalQueryResponse()`. +- WebSocket handshake/fallback logic already explicitly falls back to `startSSE()` after timeout, so SSE is a live path that must stay behaviorally aligned with WebSocket. +- Baseline backend tests currently pass with `go test ./...`. + +## Proposed approach +1. Extract one shared helper for terminal output sanitization so all output paths use the same filtering rules. +2. Split "sanitize text" from "decode bytes" so the WebSocket binary path preserves byte-stream semantics instead of per-frame UTF-8 decoding. +3. Reuse the shared sanitization helper in the SSE path and existing log replay paths to keep live, replay, and fallback behavior consistent. +4. Validate with existing Go tests plus targeted manual browser verification around UTF-8 output, instance switching, and WebSocket→SSE fallback. + +## Todos +1. Audit and refactor terminal output handling in `internal/ui/static/index.html`. +2. Introduce a persistent decoder or equivalent byte-safe handling for WebSocket binary frames. +3. Route SSE chunks through the same sanitization helper used by log replay / WebSocket text output. +4. Run repository validation (`go test ./...`) and confirm formatting impact is nil for the touched files. +5. Manually verify Chinese/multibyte output, TUI redraw, instance switching, and handshake-timeout fallback behavior. + +## Notes +- The fix should stay frontend-only unless investigation reveals backend framing assumptions need adjustment. +- Prefer a minimal, behavior-safe refactor: centralize the regex filter once, then wire each path through it. +- Be careful not to change the existing handshake/state-machine behavior while fixing output handling. diff --git a/internal/ui/static/index.html b/internal/ui/static/index.html index 7dca310..35589a1 100644 --- a/internal/ui/static/index.html +++ b/internal/ui/static/index.html @@ -477,6 +477,7 @@

Start Instance

let ttyReconnectTimer = null; let termInputBuffer = ""; let termSendTimer = null; + let ttyOutputDecoder = null; // API function rememberServerRevision(res) { @@ -945,6 +946,40 @@

Start Instance

let ttyState = 'IDLE'; // IDLE | CONNECTING | HANDSHAKING | READY | DISCONNECTING let ttyStateCheckInterval = null; // For waiting state transitions + function resetTTYOutputDecoder() { + ttyOutputDecoder = null; + } + + function sanitizeTerminalOutput(text) { + if (!text) return ""; + text = text.replace(/\x1b?\]11;[^\x07\x1b]*(\x07|\x1b\\|($|[\r\n]))/g, ''); + text = text.replace(/\x1b?\]4;[^\x07\x1b]*(\x07|\x1b\\|($|[\r\n]))/g, ''); + text = text.replace(/\x1b?\](\d+;)?rgb:[^\x07\x1b\r\n]+/g, ''); + text = text.replace(/\x1b?\[\?[\d;]*\$y/g, ''); + text = text.replace(/\x1b?\[\?[\d;]+c/g, ''); + text = text.replace(/\x1b?\[;?\d+(;\d+)?R/g, ''); + text = text.replace(/;?\d+(;\d+)*;rgb:[^\r\n]+/g, ''); + text = text.replace(/;?\d+(;\d+)+\$y/g, ''); + text = text.replace(/\d+;\d+\$y/g, ''); + text = text.replace(/;?\d+(;\d+)*R(?!\w)/g, ''); + text = text.replace(/\d+;\d+c(?!\w)/g, ''); + return text; + } + + function writeSanitizedTerminalOutput(text) { + if (!term) return; + text = sanitizeTerminalOutput(text); + if (text) term.write(text); + } + + function decodeTTYOutputChunk(bytes) { + if (!bytes || !bytes.length) return ""; + if (!ttyOutputDecoder) { + ttyOutputDecoder = new TextDecoder(); + } + return ttyOutputDecoder.decode(bytes, { stream: true }); + } + function focusTerminalIfPossible() { if (!term) return; if (!windowHasFocus) return; @@ -1180,25 +1215,7 @@

Start Instance

// Only reset for stopped instances term.reset(); } - - // Filter out terminal query responses from ALL log content - // These appear after TUI programs exit and confuse the shell - // OSC responses (color queries) - with or without ESC prefix - text = text.replace(/\x1b?\]11;[^\x07\x1b]*(\x07|\x1b\\|($|[\r\n]))/g, ''); - text = text.replace(/\x1b?\]4;[^\x07\x1b]*(\x07|\x1b\\|($|[\r\n]))/g, ''); - text = text.replace(/\x1b?\](\d+;)?rgb:[^\x07\x1b\r\n]+/g, ''); - // CSI responses - with or without ESC prefix - text = text.replace(/\x1b?\[\?[\d;]*\$y/g, ''); // DEC mode report - text = text.replace(/\x1b?\[\?[\d;]+c/g, ''); // Device attributes - text = text.replace(/\x1b?\[;?\d+(;\d+)?R/g, ''); // Cursor position report - // Standalone responses (missing ESC and [) - text = text.replace(/;?\d+(;\d+)*;rgb:[^\r\n]+/g, ''); // Partial OSC - text = text.replace(/;?\d+(;\d+)+\$y/g, ''); // Partial DEC mode (10;16;2$y) - text = text.replace(/\d+;\d+\$y/g, ''); // Standalone DEC mode (2027;0$y) - text = text.replace(/;?\d+(;\d+)*R(?!\w)/g, ''); // Partial cursor report - text = text.replace(/\d+;\d+c(?!\w)/g, ''); // Standalone Device Attr (1;2c) - - term.write(text); + writeSanitizedTerminalOutput(text); // Check if stopped and append banner if (inst && inst.status !== 'running') { @@ -1216,27 +1233,13 @@

Start Instance

if (!res.ok) throw new Error(); let text = await res.text(); const next = parseInt(res.headers.get("X-Log-Offset") || "0", 10); - // Filter terminal query responses from ALL incremental log updates - // Same filtering as loadLog() for consistency - if (text) { - text = text.replace(/\x1b?\]11;[^\x07\x1b]*(\x07|\x1b\\|($|[\r\n]))/g, ''); - text = text.replace(/\x1b?\]4;[^\x07\x1b]*(\x07|\x1b\\|($|[\r\n]))/g, ''); - text = text.replace(/\x1b?\](\d+;)?rgb:[^\x07\x1b\r\n]+/g, ''); - text = text.replace(/\x1b?\[\?[\d;]*\$y/g, ''); - text = text.replace(/\x1b?\[\?[\d;]+c/g, ''); - text = text.replace(/\x1b?\[;?\d+(;\d+)?R/g, ''); - text = text.replace(/;?\d+(;\d+)*;rgb:[^\r\n]+/g, ''); - text = text.replace(/;?\d+(;\d+)+\$y/g, ''); - text = text.replace(/\d+;\d+\$y/g, ''); - text = text.replace(/;?\d+(;\d+)*R(?!\w)/g, ''); - text = text.replace(/\d+;\d+c(?!\w)/g, ''); - } - if (text && term) term.write(text); + writeSanitizedTerminalOutput(text); if (next) logCursor = next; } catch (e) { } } function disconnectTTY() { + resetTTYOutputDecoder(); // Clear any pending state check interval if (ttyStateCheckInterval) { clearInterval(ttyStateCheckInterval); @@ -1305,6 +1308,7 @@

Start Instance

let url = `${proto}://${location.host}/api/instances/tty/ws?id=${encodeURIComponent(id)}`; if (token) url += `&token=${encodeURIComponent(token)}`; + resetTTYOutputDecoder(); const ws = new WebSocket(url); ws.binaryType = "arraybuffer"; @@ -1356,35 +1360,10 @@

Start Instance

if (ev.data instanceof ArrayBuffer) { const u8 = new Uint8Array(ev.data); if (u8.length && term) { - // Decode and filter terminal query responses - let text = new TextDecoder().decode(u8); - text = text.replace(/\x1b?\]11;[^\x07\x1b]*(\x07|\x1b\\|($|[\r\n]))/g, ''); - text = text.replace(/\x1b?\]4;[^\x07\x1b]*(\x07|\x1b\\|($|[\r\n]))/g, ''); - text = text.replace(/\x1b?\](\d+;)?rgb:[^\x07\x1b\r\n]+/g, ''); - text = text.replace(/\x1b?\[\?[\d;]*\$y/g, ''); - text = text.replace(/\x1b?\[\?[\d;]+c/g, ''); - text = text.replace(/\x1b?\[;?\d+(;\d+)?R/g, ''); - text = text.replace(/;?\d+(;\d+)*;rgb:[^\r\n]+/g, ''); - text = text.replace(/;?\d+(;\d+)+\$y/g, ''); - text = text.replace(/\d+;\d+\$y/g, ''); - text = text.replace(/;?\d+(;\d+)*R(?!\w)/g, ''); - text = text.replace(/\d+;\d+c(?!\w)/g, ''); - term.write(text); + writeSanitizedTerminalOutput(decodeTTYOutputChunk(u8)); } } else if (term) { - let text = ev.data; - text = text.replace(/\x1b?\]11;[^\x07\x1b]*(\x07|\x1b\\|($|[\r\n]))/g, ''); - text = text.replace(/\x1b?\]4;[^\x07\x1b]*(\x07|\x1b\\|($|[\r\n]))/g, ''); - text = text.replace(/\x1b?\](\d+;)?rgb:[^\x07\x1b\r\n]+/g, ''); - text = text.replace(/\x1b?\[\?[\d;]*\$y/g, ''); - text = text.replace(/\x1b?\[\?[\d;]+c/g, ''); - text = text.replace(/\x1b?\[;?\d+(;\d+)?R/g, ''); - text = text.replace(/;?\d+(;\d+)*;rgb:[^\r\n]+/g, ''); - text = text.replace(/;?\d+(;\d+)+\$y/g, ''); - text = text.replace(/\d+;\d+\$y/g, ''); - text = text.replace(/;?\d+(;\d+)*R(?!\w)/g, ''); - text = text.replace(/\d+;\d+c(?!\w)/g, ''); - term.write(text); + writeSanitizedTerminalOutput(ev.data); } // After receiving first data (after ready), trigger resize for TUI redraw @@ -1405,6 +1384,7 @@

Start Instance

}; ws.onclose = () => { + resetTTYOutputDecoder(); // Clear handshake timeout on close if (handshakeTimeout) { clearTimeout(handshakeTimeout); @@ -1438,7 +1418,7 @@

Start Instance

const onLog = (ev) => { try { const msg = JSON.parse(ev.data); - if (msg.chunk && term) term.write(msg.chunk); + if (msg.chunk && term) writeSanitizedTerminalOutput(msg.chunk); if (msg.next) logCursor = msg.next; } catch (e) { } };