mirror of
https://github.com/Ark0N/Codeman.git
synced 2026-10-02 13:39:41 +02:00
fix(mobile): close audit findings — desktop focus, claim wiring, CJK setting, ESC passthrough
Adversarial post-rebase audit (11 agents) confirmed four real issues; all fixed: - Desktop tab clicks stopped focusing the terminal: handleSessionTabClick passed preserveKeyboard:false on desktop (KeyboardHandler.keyboardVisible is mobile-only state) and selectSession's ternary mapped explicit false to 'never focus', skipping the gesture-stack focus master relies on. Focus policy now lives solely in _shouldFocusTerminalForTabSwitch() (desktop: always; touch: only while the keyboard is open). - Desktop sizing claims were almost never registered: selectSession's resizes run before _connectWs, so they went over HTTP (which never claims), leaving the arbitration inert in the canonical desktop+phone scenario. ws.onopen now sends a typed resize over the fresh socket — registering the claim and syncing PTY dims after (re)connects. - throttledResize (the main window-resize path) sent untyped HTTP resizes: a rotating phone bypassed a desktop claim, and a desktop narrowing past the tablet breakpoint never released its stale claim. It now sends typed resizes, WS-first, like sendResize. - The cjkInputEnabled App Settings toggle was silently ignored on touch phones/tablets (composer only reachable via the server inputCjkForm override, while the checkbox stayed visible and saveable). The user setting is honored everywhere again; mobile keeps native-input-by- default via the cjkInputEnabled:false mobile default. - _handleCjkInput appended multi-byte ESC sequences (hardware-keyboard arrows/Home/End on the composer) to local-echo pending text, typing raw ESC bytes into the prompt on Enter; they are now forwarded to the PTY like the onData path. Its backspace path also syncs the per-session flushed Maps the way onData does, so tab-switch restore no longer resurrects deleted characters. Defensive: Session.stop() clears desktop sizing claims (a hung client's socket close can lag teardown by a ping cycle), and the claims docblock documents the WS-only tradeoff explicitly. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
This commit is contained in:
@@ -2060,6 +2060,12 @@ export class Session extends EventEmitter {
|
||||
* registers them on a desktop-typed resize and releases them on socket
|
||||
* close, so a mobile-only session (no desktop connected) keeps full control
|
||||
* of its own size — including narrowing below the spawn default.
|
||||
*
|
||||
* Deliberate tradeoff: claims are WS-only because only a socket has a
|
||||
* liveness signal. A desktop degraded to the stateless HTTP resize fallback
|
||||
* still applies its typed resizes but holds no claim, so a concurrent phone
|
||||
* can reflow it. This is cooperative UX arbitration, not a security
|
||||
* boundary — untyped (legacy/API) resizes bypass claims by design.
|
||||
*/
|
||||
private _desktopSizeClaims = new Set<symbol>();
|
||||
|
||||
@@ -2188,6 +2194,12 @@ export class Session extends EventEmitter {
|
||||
|
||||
this._clearAllTimers();
|
||||
|
||||
// Drop desktop sizing claims defensively. Sockets normally release their
|
||||
// own claim on close, but a hung client's close event can lag the session
|
||||
// teardown by up to a ping cycle — don't let a stale claim suppress
|
||||
// mobile resizes if this Session object sees any further use.
|
||||
this._desktopSizeClaims.clear();
|
||||
|
||||
// Immediately cleanup Promise callbacks to prevent orphaned references
|
||||
// during the rest of stop() processing (e.g., if mux kill times out)
|
||||
if (this.rejectPromise && !this._promptResolved) {
|
||||
|
||||
+18
-15
@@ -1838,6 +1838,11 @@ class CodemanApp {
|
||||
if (this._ws === ws) {
|
||||
this._wsReady = true;
|
||||
this._wsReconnectAttempts = 0;
|
||||
// Send a typed resize over the fresh socket: syncs PTY dims after
|
||||
// (re)connects AND registers the desktop sizing claim server-side —
|
||||
// selectSession's earlier resizes ran before this WS existed, so they
|
||||
// went over HTTP, which never claims (see ws-routes sizingToken).
|
||||
this.sendResize(sessionId)?.catch?.(() => {});
|
||||
}
|
||||
};
|
||||
|
||||
@@ -2028,12 +2033,10 @@ class CodemanApp {
|
||||
if (!cjkEl) return;
|
||||
const settings = this.loadAppSettingsFromStorage();
|
||||
const defaults = this.getDefaultSettings?.() || {};
|
||||
const isTouchTerminal =
|
||||
typeof MobileDetection !== 'undefined' &&
|
||||
MobileDetection.isTouchDevice() &&
|
||||
(MobileDetection.isSmallScreen() || MobileDetection.isMediumScreen());
|
||||
const showCjk =
|
||||
this._serverCjkOverride || (!isTouchTerminal && (settings.cjkInputEnabled ?? defaults.cjkInputEnabled ?? false));
|
||||
// Mobile defaults ship cjkInputEnabled: false (native terminal input by
|
||||
// default on touch), but an explicit user enable is honored everywhere —
|
||||
// the App Settings toggle must not be a silent no-op on phones.
|
||||
const showCjk = this._serverCjkOverride || (settings.cjkInputEnabled ?? defaults.cjkInputEnabled ?? false);
|
||||
cjkEl.classList.toggle('cjk-input-visible', !!showCjk);
|
||||
document.body.classList.toggle('cjk-input-visible', !!showCjk);
|
||||
cjkEl.style.display = showCjk ? 'block' : 'none';
|
||||
@@ -2646,11 +2649,14 @@ class CodemanApp {
|
||||
|
||||
handleSessionTabClick(event, sessionId) {
|
||||
event?.preventDefault?.();
|
||||
const preserveKeyboard = typeof KeyboardHandler !== 'undefined' && KeyboardHandler.keyboardVisible === true;
|
||||
if (!preserveKeyboard && MobileDetection.isTouchDevice()) {
|
||||
// On touch with the keyboard hidden, blur the tapped tab so switching
|
||||
// sessions doesn't pop the on-screen keyboard. Focus policy itself lives
|
||||
// in selectSession via _shouldFocusTerminalForTabSwitch().
|
||||
const keyboardOpen = typeof KeyboardHandler !== 'undefined' && KeyboardHandler.keyboardVisible === true;
|
||||
if (!keyboardOpen && MobileDetection.isTouchDevice()) {
|
||||
document.activeElement?.blur?.();
|
||||
}
|
||||
return this.selectSession(sessionId, { forceReload: true, preserveKeyboard });
|
||||
return this.selectSession(sessionId, { forceReload: true });
|
||||
}
|
||||
|
||||
|
||||
@@ -2954,12 +2960,9 @@ class CodemanApp {
|
||||
// programmatic focus() within the user-gesture call stack (e.g. tab click).
|
||||
// After the first await the gesture context is lost and focus() is silently
|
||||
// ignored, leaving the keyboard unable to send input to the terminal.
|
||||
const shouldFocusTerminal =
|
||||
options?.preserveKeyboard === true
|
||||
? this._shouldFocusTerminalForTabSwitch()
|
||||
: options?.preserveKeyboard === false
|
||||
? false
|
||||
: this._shouldFocusTerminalForTabSwitch();
|
||||
// Desktop always focuses; touch focuses only while the on-screen keyboard
|
||||
// is already open (so a tab switch doesn't pop the keyboard).
|
||||
const shouldFocusTerminal = this._shouldFocusTerminalForTabSwitch();
|
||||
if (shouldFocusTerminal && this.terminal) this.terminal.focus();
|
||||
|
||||
const _selStart = performance.now();
|
||||
|
||||
@@ -496,11 +496,30 @@ Object.assign(CodemanApp.prototype, {
|
||||
this.terminal.write('\x1b[3J\x1b[H\x1b[2J');
|
||||
}
|
||||
this._lastResizeDims = { cols, rows };
|
||||
fetch(`/api/sessions/${this.activeSessionId}/resize`, {
|
||||
method: 'POST',
|
||||
headers: { 'Content-Type': 'application/json' },
|
||||
body: JSON.stringify({ cols, rows }),
|
||||
}).catch(() => {});
|
||||
// Typed + WS-first like sendResize: the viewport type feeds resize
|
||||
// arbitration (a phone rotating must not bypass a desktop claim),
|
||||
// and a desktop window narrowing past the tablet breakpoint must
|
||||
// send a typed WS frame so its stale desktop claim is released.
|
||||
const viewportType =
|
||||
typeof MobileDetection !== 'undefined' && MobileDetection.getDeviceType
|
||||
? MobileDetection.getDeviceType()
|
||||
: 'desktop';
|
||||
let sentViaWs = false;
|
||||
if (this._wsReady && this._wsSessionId === this.activeSessionId) {
|
||||
try {
|
||||
this._ws.send(JSON.stringify({ t: 'z', c: cols, r: rows, v: viewportType }));
|
||||
sentViaWs = true;
|
||||
} catch {
|
||||
// Fall through to HTTP POST
|
||||
}
|
||||
}
|
||||
if (!sentViaWs) {
|
||||
fetch(`/api/sessions/${this.activeSessionId}/resize`, {
|
||||
method: 'POST',
|
||||
headers: { 'Content-Type': 'application/json' },
|
||||
body: JSON.stringify({ cols, rows, viewportType }),
|
||||
}).catch(() => {});
|
||||
}
|
||||
}
|
||||
}
|
||||
// Update subagent connection lines and local echo at new dimensions
|
||||
@@ -1519,7 +1538,22 @@ Object.assign(CodemanApp.prototype, {
|
||||
|
||||
if (text === '\x7f') {
|
||||
const source = this._localEchoOverlay.removeChar();
|
||||
if (source === 'flushed') this._sendInputAsync(sessionId, text);
|
||||
if (source === 'flushed') {
|
||||
// Sync app-level flushed Maps (per-session state for tab switching),
|
||||
// mirroring the onData backspace path — otherwise switching tabs away
|
||||
// and back restores a stale, too-long flushed overlay.
|
||||
const { count, text: flushedText } = this._localEchoOverlay.getFlushed();
|
||||
if (this._flushedOffsets?.has(sessionId)) {
|
||||
if (count === 0) {
|
||||
this._flushedOffsets.delete(sessionId);
|
||||
this._flushedTexts?.delete(sessionId);
|
||||
} else {
|
||||
this._flushedOffsets.set(sessionId, count);
|
||||
this._flushedTexts?.set(sessionId, flushedText);
|
||||
}
|
||||
}
|
||||
this._sendInputAsync(sessionId, text);
|
||||
}
|
||||
return;
|
||||
}
|
||||
|
||||
@@ -1536,6 +1570,15 @@ Object.assign(CodemanApp.prototype, {
|
||||
return;
|
||||
}
|
||||
|
||||
// Multi-byte escape sequence (arrow/Home/End from a hardware keyboard on
|
||||
// the composer) — forward to the PTY without touching overlay state,
|
||||
// mirroring the onData path. Appending it to pending text would type raw
|
||||
// ESC bytes into the prompt on the next Enter.
|
||||
if (text.length > 1 && text.charCodeAt(0) === 27) {
|
||||
this._sendInputAsync(sessionId, text);
|
||||
return;
|
||||
}
|
||||
|
||||
if (text.length === 1 && text.charCodeAt(0) < 32) {
|
||||
const pending = this._localEchoOverlay.pendingText || '';
|
||||
this._localEchoOverlay.clear();
|
||||
|
||||
Reference in New Issue
Block a user