diff --git a/web/src/lib/pty-mobile-input.test.ts b/web/src/lib/pty-mobile-input.test.ts index 85ba22007ca..5245a2e3186 100644 --- a/web/src/lib/pty-mobile-input.test.ts +++ b/web/src/lib/pty-mobile-input.test.ts @@ -59,6 +59,15 @@ describe("normalizePtyMobileInput", () => { expect(result.nextLine).toBe(""); expect(result.data).toBe("\r"); }); + + it("does not collapse legitimate single-letter reduplication", () => { + // "a a" is a plausible thing to type; the >=2-char guard keeps the + // duplicate-final-word collapse from eating it inside the window. + const result = normalizePtyMobileInput("a a", "a", true); + + expect(result.normalized).toBe(false); + expect(result.data).toBe("a a"); + }); }); describe("updatePtyInputLine", () => { diff --git a/web/src/lib/pty-mobile-input.ts b/web/src/lib/pty-mobile-input.ts index aaee4c88cbc..d6eb024101a 100644 --- a/web/src/lib/pty-mobile-input.ts +++ b/web/src/lib/pty-mobile-input.ts @@ -1,5 +1,10 @@ const DELETE = "\x7f"; +// How long (ms) after a mobile IME / replacement event we treat subsequent +// terminal input as a candidate line-replacement rather than a plain append. +// Exported so the ChatPage integration and tests share one tunable value. +export const MOBILE_REPLACEMENT_WINDOW_MS = 350; + function chars(text: string): string[] { return Array.from(text); } @@ -24,6 +29,11 @@ function collapseDuplicatedFinalWord(text: string, previousLine: string): string const [, prefix, first, , second, trailing] = match; if (first.toLocaleLowerCase() !== second.toLocaleLowerCase()) return text; + // Only collapse a duplication the tracked line already ended with — i.e. + // Gboard re-emitted the final word. Requiring a >=2-char word avoids + // eating legitimate single-letter reduplication ("a a", "i i") that a + // user may genuinely type inside the replacement window. + if (first.length < 2) return text; if (!previousLine.trimEnd().toLocaleLowerCase().endsWith(first.toLocaleLowerCase())) { return text; } diff --git a/web/src/lib/pty-reconnect.test.ts b/web/src/lib/pty-reconnect.test.ts index 3e8fc40648f..48543a65402 100644 --- a/web/src/lib/pty-reconnect.test.ts +++ b/web/src/lib/pty-reconnect.test.ts @@ -79,6 +79,37 @@ describe("shouldReconnectPtyOnPageResume", () => { }), ).toBe(false); }); + + it("does not fire a redundant reconnect while a connect is in flight (wsRef not yet assigned)", () => { + // The async socket-open IIFE has begun but not yet assigned wsRef, so + // socketReadyState reads null. Without the connectInFlight guard this + // would return true and double-connect. + expect( + shouldReconnectPtyOnPageResume({ + isActive: true, + visibilityState: "visible", + online: true, + socketReadyState: null, + ptyState: "connecting", + connectInFlight: true, + }), + ).toBe(false); + }); + + it("still reconnects an in-flight connect when the page already believes it is closed", () => { + // A stuck attempt the user is actively trying to recover (manual reconnect + // or a closed state) must not be suppressed by the in-flight guard. + expect( + shouldReconnectPtyOnPageResume({ + isActive: true, + visibilityState: "visible", + online: true, + socketReadyState: null, + ptyState: "closed", + connectInFlight: true, + }), + ).toBe(true); + }); }); describe("shouldBlockPtyInput", () => { diff --git a/web/src/lib/pty-reconnect.ts b/web/src/lib/pty-reconnect.ts index 51bd3bdf38b..8af050493dd 100644 --- a/web/src/lib/pty-reconnect.ts +++ b/web/src/lib/pty-reconnect.ts @@ -8,12 +8,23 @@ export type PtyConnectionState = export const PTY_RECONNECT_INPUT_MESSAGE = "Chat is reconnecting. Input will resume when connected."; +// Minimum gap (ms) between page-resume-triggered reconnect attempts, so a +// burst of visibilitychange/pageshow/focus/online events on tab-return +// collapses into a single reconnect. +export const PTY_RESUME_RECONNECT_THROTTLE_MS = 1000; + +// If a socket sits in WS_CONNECTING past this budget it is treated as wedged +// (e.g. a half-open mobile socket after a radio handoff — the NS-591 case) +// and force-closed so `onclose` → scheduleReconnect can recover it. +export const PTY_CONNECTING_TIMEOUT_MS = 8000; + export interface PtyResumeReconnectInput { isActive: boolean; visibilityState?: DocumentVisibilityState; online: boolean; socketReadyState?: number | null; ptyState: PtyConnectionState; + connectInFlight?: boolean; } const WS_CONNECTING = 0; @@ -27,6 +38,7 @@ export function shouldReconnectPtyOnPageResume({ online, socketReadyState, ptyState, + connectInFlight, }: PtyResumeReconnectInput): boolean { if (!isActive || !online || visibilityState === "hidden") { return false; @@ -37,8 +49,13 @@ export function shouldReconnectPtyOnPageResume({ if (socketReadyState === WS_OPEN) { return false; } + // A connect is mid-flight (the async socket-open IIFE is awaiting its + // ticket URL and hasn't assigned wsRef yet, or the socket is still + // CONNECTING on a non-stuck attempt). Don't fire a redundant reconnect + // into that window unless the tab already believes it is reconnecting or + // closed and needs a fresh attempt. if ( - socketReadyState === WS_CONNECTING && + (connectInFlight || socketReadyState === WS_CONNECTING) && ptyState !== "reconnecting" && ptyState !== "closed" ) { @@ -47,7 +64,6 @@ export function shouldReconnectPtyOnPageResume({ return ( socketReadyState === null || socketReadyState === undefined || - socketReadyState === WS_CONNECTING || socketReadyState === WS_CLOSING || socketReadyState === WS_CLOSED || ptyState === "reconnecting" || diff --git a/web/src/pages/ChatPage.tsx b/web/src/pages/ChatPage.tsx index 9d7ef5c149e..68f9221aaa4 100644 --- a/web/src/pages/ChatPage.tsx +++ b/web/src/pages/ChatPage.tsx @@ -37,12 +37,15 @@ import { useI18n } from "@/i18n"; import { api } from "@/lib/api"; import { normalizeSessionTitle } from "@/lib/chat-title"; import { + PTY_CONNECTING_TIMEOUT_MS, PTY_RECONNECT_INPUT_MESSAGE, + PTY_RESUME_RECONNECT_THROTTLE_MS, type PtyConnectionState, shouldBlockPtyInput, shouldReconnectPtyOnPageResume, } from "@/lib/pty-reconnect"; import { + MOBILE_REPLACEMENT_WINDOW_MS, normalizePtyMobileInput, shouldTreatInputAsMobileReplacement, } from "@/lib/pty-mobile-input"; @@ -172,6 +175,11 @@ export default function ChatPage({ isActive = true }: { isActive?: boolean }) { const forceFreshPtyRef = useRef(false); const blockedInputNoticeRef = useRef(false); const lastResumeReconnectAtRef = useRef(0); + // True from the moment the connect effect begins until the socket resolves + // (open or close). Guards the page-resume reconnect against firing during + // the async ticket/URL await gap where wsRef.current is not yet assigned. + const connectInFlightRef = useRef(false); + const connectingTimerRef = useRef | null>(null); const ptyInputLineRef = useRef(""); const mobileReplacementInputUntilRef = useRef(0); const [ptyState, setPtyState] = @@ -618,11 +626,11 @@ export default function ChatPage({ isActive = true }: { isActive?: boolean }) { isMobileLike, ) ) { - mobileReplacementInputUntilRef.current = Date.now() + 350; + mobileReplacementInputUntilRef.current = Date.now() + MOBILE_REPLACEMENT_WINDOW_MS; } }; const markCompositionEnd = () => { - mobileReplacementInputUntilRef.current = Date.now() + 350; + mobileReplacementInputUntilRef.current = Date.now() + MOBILE_REPLACEMENT_WINDOW_MS; }; textarea.addEventListener("beforeinput", markReplacementInput, true); @@ -761,6 +769,16 @@ export default function ChatPage({ isActive = true }: { isActive?: boolean }) { let onResizeDisposable: { dispose(): void } | null = null; const forceFresh = forceFreshPtyRef.current; forceFreshPtyRef.current = false; + // A connect attempt is now in flight — set synchronously (before the async + // socket-open IIFE below awaits its ticket URL) so a page-resume event in + // that gap doesn't fire a redundant reconnect (wsRef isn't assigned yet). + connectInFlightRef.current = true; + const clearConnectingTimer = () => { + if (connectingTimerRef.current) { + clearTimeout(connectingTimerRef.current); + connectingTimerRef.current = null; + } + }; const scheduleReconnect = (code: number) => { if (reconnectTimerRef.current) { return; @@ -793,9 +811,26 @@ export default function ChatPage({ isActive = true }: { isActive?: boolean }) { const ws = new WebSocket(url); ws.binaryType = "arraybuffer"; wsRef.current = ws; + // W2 (NS-591): a mobile socket can wedge in CONNECTING after a radio + // handoff and never fire onclose, so neither the resume predicate nor + // scheduleReconnect can recover it. Force-close if it hasn't opened + // within the budget; the resulting onclose routes into scheduleReconnect. + clearConnectingTimer(); + connectingTimerRef.current = setTimeout(() => { + connectingTimerRef.current = null; + if (wsRef.current === ws && ws.readyState === WebSocket.CONNECTING) { + try { + ws.close(); + } catch { + /* already tearing down */ + } + } + }, PTY_CONNECTING_TIMEOUT_MS); ws.onopen = () => { clearReconnectTimer(); + clearConnectingTimer(); + connectInFlightRef.current = false; reconnectAttemptRef.current = 0; setBanner(null); setLastCloseCode(null); @@ -842,6 +877,8 @@ export default function ChatPage({ isActive = true }: { isActive?: boolean }) { ws.onclose = (ev) => { wsRef.current = null; + connectInFlightRef.current = false; + clearConnectingTimer(); if (unmounting) { return; } @@ -998,6 +1035,8 @@ export default function ChatPage({ isActive = true }: { isActive?: boolean }) { if (settleRaf1) cancelAnimationFrame(settleRaf1); if (settleRaf2) cancelAnimationFrame(settleRaf2); clearReconnectTimer(); + clearConnectingTimer(); + connectInFlightRef.current = false; // Phase 5.3: ``ws`` is local to the IIFE that opens it (the gated-mode // ticket fetch makes the open async). The cleanup runs at the outer // effect's top level so it can't reach into that scope — close via @@ -1082,10 +1121,11 @@ export default function ChatPage({ isActive = true }: { isActive?: boolean }) { online, socketReadyState, ptyState: ptyStateRef.current, + connectInFlight: connectInFlightRef.current, }) ) { const now = Date.now(); - if (now - lastResumeReconnectAtRef.current < 1000) { + if (now - lastResumeReconnectAtRef.current < PTY_RESUME_RECONNECT_THROTTLE_MS) { return; } lastResumeReconnectAtRef.current = now;