Skip to content

fix(ui): refactor terminal output handling for UTF-8 and SSE consistency #19

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
linletian merged 1 commit into from
Mar 13, 2026
Merged

fix(ui): refactor terminal output handling for UTF-8 and SSE consistency #19

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
32 changes: 32 additions & 0 deletions docs/pr18-fix-plan.md
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -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.
106 changes: 43 additions & 63 deletions internal/ui/static/index.html
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -477,6 +477,7 @@ <h3>Start Instance</h3>
let ttyReconnectTimer = null;
let termInputBuffer = "";
let termSendTimer = null;
let ttyOutputDecoder = null;

// API
function rememberServerRevision(res) {
Expand Down Expand Up @@ -945,6 +946,40 @@ <h3>Start Instance</h3>
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;
Expand Down Expand Up @@ -1180,25 +1215,7 @@ <h3>Start Instance</h3>
// 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') {
Expand All @@ -1216,27 +1233,13 @@ <h3>Start Instance</h3>
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);
Expand Down Expand Up @@ -1305,6 +1308,7 @@ <h3>Start Instance</h3>
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";

Expand Down Expand Up @@ -1356,35 +1360,10 @@ <h3>Start Instance</h3>
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
Expand All @@ -1405,6 +1384,7 @@ <h3>Start Instance</h3>
};

ws.onclose = () => {
resetTTYOutputDecoder();
// Clear handshake timeout on close
if (handshakeTimeout) {
clearTimeout(handshakeTimeout);
Expand Down Expand Up @@ -1438,7 +1418,7 @@ <h3>Start Instance</h3>
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) { }
};
Expand Down