diff --git a/studio/frontend/src/components/assistant-ui/thread.tsx b/studio/frontend/src/components/assistant-ui/thread.tsx index ff80099505..d72547aa1a 100644 --- a/studio/frontend/src/components/assistant-ui/thread.tsx +++ b/studio/frontend/src/components/assistant-ui/thread.tsx @@ -80,6 +80,7 @@ import { type CompositionEvent, type FC, type FormEvent, + type KeyboardEvent, useCallback, useEffect, useRef, @@ -353,16 +354,58 @@ function isNativeComposing(event: Event) { return "isComposing" in event && (event as InputEvent).isComposing === true; } +// Fallback timeout for stuck IME composition. When Chrome on Windows talks +// to a WSL-hosted Studio (issue #5546), `compositionend` never fires after +// the candidate is committed, so `composingRef` stays true and Send stays +// disabled. Every compositionupdate / non-composing input resets the timer; +// only a true gap-after-commit lets it fire. 2500ms is well above a normal +// candidate-window pause but short enough to recover before the user +// notices the Send button is stuck. +const IME_STUCK_TIMEOUT_MS = 2500; + function useImeComposerInputHandlers() { const aui = useAui(); const composingRef = useRef(false); const [isComposing, setIsComposing] = useState(false); + const stuckTimerRef = useRef | null>(null); - const setCompositionState = useCallback((next: boolean) => { - composingRef.current = next; - setIsComposing(next); + const clearStuckTimer = useCallback(() => { + if (stuckTimerRef.current) { + clearTimeout(stuckTimerRef.current); + stuckTimerRef.current = null; + } }, []); + const setCompositionState = useCallback( + (next: boolean) => { + composingRef.current = next; + setIsComposing(next); + clearStuckTimer(); + if (next) { + stuckTimerRef.current = setTimeout(() => { + stuckTimerRef.current = null; + composingRef.current = false; + setIsComposing(false); + }, IME_STUCK_TIMEOUT_MS); + } + }, + [clearStuckTimer], + ); + + const refreshStuckTimer = useCallback(() => { + if (!composingRef.current) { + return; + } + clearStuckTimer(); + stuckTimerRef.current = setTimeout(() => { + stuckTimerRef.current = null; + composingRef.current = false; + setIsComposing(false); + }, IME_STUCK_TIMEOUT_MS); + }, [clearStuckTimer]); + + useEffect(() => clearStuckTimer, [clearStuckTimer]); + const setComposerText = useCallback( (value: string) => { const composer = aui.composer(); @@ -380,6 +423,10 @@ function useImeComposerInputHandlers() { setCompositionState(true); }, [setCompositionState]); + const onCompositionUpdate = useCallback(() => { + refreshStuckTimer(); + }, [refreshStuckTimer]); + const onCompositionEnd = useCallback( (e: CompositionEvent) => { setCompositionState(false); @@ -396,11 +443,31 @@ function useImeComposerInputHandlers() { [setComposerText, setCompositionState], ); + // If the watchdog cleared the composing flags during a long candidate-window + // pause, a subsequent IME keypress (browser-side isComposing=true / IME + // keyCode 229) would otherwise reach handleSubmit with composingRef=false + // and submit the preedit text. Re-arm composingRef synchronously from the + // native event so the form-submit gate keeps blocking until compositionend. + // Re-arm the watchdog at the same time — otherwise the WSL+Chrome path + // this PR targets (no compositionend, no follow-up input event) would + // leave composingRef pinned true indefinitely and Send blocked again. + const onKeyDown = useCallback( + (e: KeyboardEvent) => { + if (e.nativeEvent.isComposing || e.keyCode === 229) { + composingRef.current = true; + refreshStuckTimer(); + } + }, + [refreshStuckTimer], + ); + return { inputProps: { onCompositionStart, + onCompositionUpdate, onCompositionEnd, onChange, + onKeyDown, }, isComposing, isComposingRef: composingRef, diff --git a/studio/frontend/src/features/chat/shared-composer.tsx b/studio/frontend/src/features/chat/shared-composer.tsx index c320f6d86b..aef004e891 100644 --- a/studio/frontend/src/features/chat/shared-composer.tsx +++ b/studio/frontend/src/features/chat/shared-composer.tsx @@ -68,6 +68,11 @@ function isNativeComposing(event: Event) { return "isComposing" in event && (event as InputEvent).isComposing === true; } +// Mirrors the threshold in thread.tsx — see the comment there. Chrome on +// Windows-over-WSL (issue #5546) never fires `compositionend` after the +// IME commit, so the compose flag would otherwise stay true forever. +const IME_STUCK_TIMEOUT_MS = 2500; + function fileToBase64DataURL(file: File): Promise { return new Promise((resolve, reject) => { const reader = new FileReader(); @@ -284,6 +289,7 @@ export function SharedComposer({ const [isComposing, setIsComposing] = useState(false); const textareaRef = useRef(null); const composingRef = useRef(false); + const stuckImeTimerRef = useRef | null>(null); const fileInputRef = useRef(null); const audioInputRef = useRef(null); @@ -474,11 +480,40 @@ export function SharedComposer({ setPendingImages((prev) => prev.filter((p) => p.id !== id)); }, []); + function clearStuckImeTimer() { + if (stuckImeTimerRef.current) { + clearTimeout(stuckImeTimerRef.current); + stuckImeTimerRef.current = null; + } + } + function setCompositionState(next: boolean) { composingRef.current = next; setIsComposing(next); + clearStuckImeTimer(); + if (next) { + stuckImeTimerRef.current = setTimeout(() => { + stuckImeTimerRef.current = null; + composingRef.current = false; + setIsComposing(false); + }, IME_STUCK_TIMEOUT_MS); + } } + function refreshStuckImeTimer() { + if (!composingRef.current) { + return; + } + clearStuckImeTimer(); + stuckImeTimerRef.current = setTimeout(() => { + stuckImeTimerRef.current = null; + composingRef.current = false; + setIsComposing(false); + }, IME_STUCK_TIMEOUT_MS); + } + + useEffect(() => () => clearStuckImeTimer(), []); + async function send() { if (composingRef.current) return; const msg = text.trim(); @@ -682,8 +717,17 @@ export function SharedComposer({ function onKeyDown(e: KeyboardEvent) { // IME composition (Japanese/Chinese/Korean): Enter commits the candidate. - // Don't hijack it. See issue #5318. - if (e.nativeEvent.isComposing || e.keyCode === 229) return; + // Don't hijack it. See issue #5318. Re-pin composingRef in case the stuck + // watchdog (#5546) cleared it during a long candidate-window pause; this + // keeps a follow-up click-Send from submitting preedit text. Re-arm the + // watchdog on the same path — without it the WSL+Chrome no-compositionend + // case would leave composingRef pinned forever after an IME keypress and + // re-lock Send. + if (e.nativeEvent.isComposing || e.keyCode === 229) { + composingRef.current = true; + refreshStuckImeTimer(); + return; + } if (e.key === "Enter" && !e.shiftKey) { e.preventDefault(); if (!busy) { @@ -753,6 +797,9 @@ export function SharedComposer({ onCompositionStart={() => { setCompositionState(true); }} + onCompositionUpdate={() => { + refreshStuckImeTimer(); + }} onCompositionEnd={(e: CompositionEvent) => { setCompositionState(false); setText(e.currentTarget.value); diff --git a/tests/studio/playwright_chat_ime_i18n.py b/tests/studio/playwright_chat_ime_i18n.py index c882d88cbd..efcd048b44 100644 --- a/tests/studio/playwright_chat_ime_i18n.py +++ b/tests/studio/playwright_chat_ime_i18n.py @@ -3,12 +3,16 @@ """Studio chat composer IME + multilingual regression smoke. -Covers two surfaces: +Covers three surfaces: A. Stuck IME composition (issue #5318 / PR #5327): duplicate compositionstart with no compositionend left isComposing=true, dropping all subsequent keystrokes including ASCII. B. Multilingual paste round-trip across 31 scripts -- guards the controlled-textarea / React state plumbing against Unicode mangling. + C. Stuck compositionend (issue #5546): Chrome on Windows over WSL + fires compositionstart + compositionupdate but never compositionend, + wedging Send disabled after the IME commits. Verifies the + watchdog in useImeComposerInputHandlers releases the flag. Model-free; the bug surface is the composer, not inference. @@ -424,6 +428,195 @@ with sync_playwright() as p: info("stuck-composition recovery PASS") clear() + # 6b. WSL + Windows Chrome repro for issue #5546: Chrome never emits + # compositionend after the IME commit, so the watchdog has to + # release the composing flag on its own once the events go silent. + # This dispatches a realistic "compose, commit, then nothing" + # sequence — no compositionend, no follow-up keystrokes — and + # waits for the Send button to come back enabled. + step("BUG REPRO: stuck compositionend recovery (issue #5546)") + clear() + composer.click() + composer.evaluate( + """(el) => { + el.focus(); + el.dispatchEvent(new CompositionEvent('compositionstart', {bubbles:true, data:''})); + el.dispatchEvent(new CompositionEvent('compositionupdate', {bubbles:true, data:'你'})); + el.dispatchEvent(new CompositionEvent('compositionupdate', {bubbles:true, data:'你好'})); + const setter = Object.getOwnPropertyDescriptor( + window.HTMLTextAreaElement.prototype, 'value' + ).set; + setter.call(el, el.value + '你好'); + el.dispatchEvent(new InputEvent('input', { + bubbles:true, inputType:'insertCompositionText', + data:'你好', isComposing:true, + })); + // Deliberately omit compositionend — that is the WSL/Chrome + // bug surface. The watchdog in useImeComposerInputHandlers + // should reset isComposing after IME_STUCK_TIMEOUT_MS. + }""" + ) + send_btn_5546 = page.locator('button[aria-label="Send message"]') + if send_btn_5546.count() == 0: + soft_fail("Send button not found for #5546 repro") + else: + # Watchdog is 2500ms; allow generous slack for slow CI. + try: + expect(send_btn_5546).not_to_be_disabled(timeout = 8_000) + info("Send button enabled after compositionend never fired") + except Exception: + shoot("06b-compositionend-watchdog-FAIL") + fail( + "Send button stayed disabled with no compositionend — " + "watchdog did not release the composing flag (issue #5546)." + ) + after_value = read_value() + if "你好" not in after_value: + soft_fail(f"compositionend-watchdog repro lost committed text: {after_value!r}") + shoot("06b-compositionend-watchdog") + info("compositionend watchdog recovery PASS") + clear() + + # 6c. Watchdog-race repro: after the watchdog clears composingRef during a + # long candidate pause, a subsequent IME keydown (browser still sees + # isComposing=true / keyCode 229) must not slip preedit text through + # the form submit. The onKeyDown gate re-pins composingRef so the + # handleSubmit / blockSend guards keep refusing. The Send button stays + # visually enabled (watchdog has already cleared the React state); the + # refusal happens at form.requestSubmit() time, not at the button. + step( + "BUG REPRO: keydown re-pin after watchdog cleared composing (issue #5546 follow-up)" + ) + clear() + composer.click() + composer.evaluate( + """(el) => { + el.focus(); + el.dispatchEvent(new CompositionEvent('compositionstart', {bubbles:true, data:''})); + el.dispatchEvent(new CompositionEvent('compositionupdate', {bubbles:true, data:'半'})); + const setter = Object.getOwnPropertyDescriptor( + window.HTMLTextAreaElement.prototype, 'value' + ).set; + setter.call(el, el.value + '半角'); + el.dispatchEvent(new InputEvent('input', { + bubbles:true, inputType:'insertCompositionText', + data:'半角', isComposing:true, + })); + }""" + ) + send_btn_keydown = page.locator('button[aria-label="Send message"]') + # Wait past the watchdog so composingRef has cleared. + try: + expect(send_btn_keydown).not_to_be_disabled(timeout = 8_000) + except Exception: + soft_fail("watchdog did not clear before keydown re-pin test") + # Fire the IME-confirm Enter (keyCode 229, isComposing=true) then trigger + # the form submit synchronously. With the keydown gate, composingRef is + # re-pinned before handleSubmit runs and the submit is prevented; the + # textarea must still hold the preedit text. + submit_probe = composer.evaluate( + """(el) => { + el.focus(); + el.dispatchEvent(new KeyboardEvent('keydown', { + bubbles:true, key:'Enter', code:'Enter', keyCode:229, + isComposing:true, + })); + const form = el.closest('form'); + const before = el.value; + try { form && form.requestSubmit(); } catch (e) {} + return {before, after: el.value, cleared: before !== '' && el.value === ''}; + }""" + ) + if submit_probe.get("cleared"): + shoot("06c-keydown-repin-FAIL") + fail( + "Form submitted after an IME keydown -- preedit text leaked " + "through the watchdog gap (#5546 follow-up regression)." + ) + info( + f"Form submit refused after IME keydown; textarea retained {submit_probe.get('after')!r}" + ) + shoot("06c-keydown-repin") + info("keydown re-pin gate PASS") + clear() + + # 6d. Keydown re-pin must also re-arm the watchdog. On the WSL+Chrome + # stuck-compositionend path the IME never fires a follow-up + # compositionend or non-composing input, so after the IME keydown + # re-pins composingRef the watchdog has to take it back to false on + # its own — otherwise Send re-locks permanently after the very + # scenario this PR was supposed to fix. (Codex P1, commit 597af0d0.) + step("BUG REPRO: keydown re-pin re-arms watchdog (#5546 follow-up regression)") + clear() + composer.click() + composer.evaluate( + """(el) => { + el.focus(); + el.dispatchEvent(new CompositionEvent('compositionstart', {bubbles:true, data:''})); + el.dispatchEvent(new CompositionEvent('compositionupdate', {bubbles:true, data:'你'})); + const setter = Object.getOwnPropertyDescriptor( + window.HTMLTextAreaElement.prototype, 'value' + ).set; + setter.call(el, el.value + '你好'); + el.dispatchEvent(new InputEvent('input', { + bubbles:true, inputType:'insertCompositionText', + data:'你好', isComposing:true, + })); + }""" + ) + send_btn_rearm = page.locator('button[aria-label="Send message"]') + # First watchdog cycle: wait for it to clear composingRef. + try: + expect(send_btn_rearm).not_to_be_disabled(timeout = 8_000) + except Exception: + soft_fail("watchdog did not clear before re-arm test (first cycle)") + # IME-confirm keydown re-pins composingRef. Without the re-arm fix the + # watchdog would never run again and Send would stay blocked at the + # submit-time guard forever, even though no follow-up IME event arrives. + composer.evaluate( + """(el) => { + el.focus(); + el.dispatchEvent(new KeyboardEvent('keydown', { + bubbles:true, key:'Enter', code:'Enter', keyCode:229, + isComposing:true, + })); + }""" + ) + # Second watchdog cycle: a real submit attempt now must eventually be + # allowed. Trigger requestSubmit() after the re-armed watchdog window + # plus a little slack; on the buggy build the form stays gated forever. + rearm_probe = page.evaluate( + """async (selector) => { + const ta = document.querySelector(selector); + const form = ta && ta.closest('form'); + if (!form || !ta) return {ok: false, reason: 'composer missing'}; + const before = ta.value; + // Wait past the 2500ms watchdog + slack so the re-armed timer + // fires. If the fix is missing this still resolves but the + // submit will not flush the textarea. + await new Promise(r => setTimeout(r, 3500)); + try { form.requestSubmit(); } catch (e) {} + // Give the submit handler a tick to flush state. + await new Promise(r => setTimeout(r, 250)); + return {ok: true, before, after: ta.value}; + }""", + 'textarea[aria-label="Message input"]', + ) + if rearm_probe.get("ok") and rearm_probe.get("after") == rearm_probe.get("before"): + shoot("06d-keydown-rearm-FAIL") + fail( + "After the keydown re-pin the watchdog never re-armed; Send " + "stayed permanently locked on the WSL+Chrome stuck-end path " + "(#5546 follow-up Codex P1)." + ) + info( + "watchdog re-armed after keydown re-pin: textarea flushed from " + f"{rearm_probe.get('before')!r} to {rearm_probe.get('after')!r}" + ) + shoot("06d-keydown-rearm") + info("keydown re-pin re-arm PASS") + clear() + # 7. Final state. The change-password redirect emits benign 401 noise, # so we filter via is_benign_* and only fail on real errors. shoot("07-final") @@ -451,7 +644,9 @@ with sync_playwright() as p: info( f"DONE: ascii=OK paste={len(I18N_SAMPLES)}/{len(I18N_SAMPLES)} " - f"normal_composition=OK stuck_recovery=OK" + f"normal_composition=OK stuck_recovery=OK " + f"compositionend_watchdog=OK keydown_repin=OK " + f"keydown_repin_rearm=OK" ) _watchdog.cancel() browser.close() diff --git a/tests/studio/test_composer_rtl_bidi_attribute.py b/tests/studio/test_composer_rtl_bidi_attribute.py index 5b1437b4fc..a1af16d4fc 100644 --- a/tests/studio/test_composer_rtl_bidi_attribute.py +++ b/tests/studio/test_composer_rtl_bidi_attribute.py @@ -71,3 +71,102 @@ def test_ime_playwright_script_does_not_read_studio_old_pw(): "STUDIO_OLD_PW" not in code_only ), "IME Playwright script still references dead STUDIO_OLD_PW env var" assert 'os.environ["STUDIO_NEW_PW"]' in code_only + + +def test_main_composer_has_stuck_compositionend_watchdog(): + """Issue #5546: Chrome on Windows over WSL never emits compositionend + after the IME commit. The composer keeps a watchdog that releases the + composing flag once events go silent; without it Send stays disabled + forever and CJK input is effectively dropped.""" + src = THREAD_TSX.read_text() + assert "IME_STUCK_TIMEOUT_MS" in src, ( + "main composer is missing the stuck-compositionend watchdog " "(issue #5546)" + ) + assert "onCompositionUpdate" in src, ( + "main composer is missing onCompositionUpdate wiring; the " + "watchdog only resets while the IME is actively emitting events" + ) + + +def test_compare_composer_has_stuck_compositionend_watchdog(): + src = SHARED_TSX.read_text() + assert "IME_STUCK_TIMEOUT_MS" in src, ( + "compare composer is missing the stuck-compositionend watchdog " "(issue #5546)" + ) + assert ( + "onCompositionUpdate" in src + ), "compare composer is missing onCompositionUpdate wiring" + + +def test_main_composer_keydown_repins_composing_during_ime(): + """Issue #5546 watchdog can clear composingRef during a long candidate + pause; the IME keydown gate must re-pin it so a follow-up Enter does not + submit preedit text.""" + src = THREAD_TSX.read_text() + assert "onKeyDown" in src, "main composer is missing onKeyDown IME gate" + assert "e.nativeEvent.isComposing" in src and "keyCode === 229" in src, ( + "main composer keydown gate must check both nativeEvent.isComposing " + "and the IME keyCode 229 sentinel" + ) + + +def test_compare_composer_keydown_repins_composing_during_ime(): + """Compare composer onKeyDown re-pins composingRef on IME keypress so a + follow-up click-Send during the watchdog window does not slip preedit + text through.""" + src = SHARED_TSX.read_text() + assert "composingRef.current = true" in src, ( + "compare composer keydown gate must re-pin composingRef when the " + "browser still considers the IME active" + ) + + +def _extract_block(src: str, anchor: str, opener: str = "(", closer: str = ")") -> str: + """Return the source between the first balanced opener/closer that + starts at or after `anchor`. Used to scope assertions to a specific + handler so a re-arm call in some other function does not satisfy + the gate test.""" + start = src.find(anchor) + assert start != -1, f"anchor {anchor!r} not found" + open_idx = src.find(opener, start) + assert open_idx != -1, f"opener {opener!r} after {anchor!r} not found" + depth = 0 + for i in range(open_idx, len(src)): + c = src[i] + if c == opener: + depth += 1 + elif c == closer: + depth -= 1 + if depth == 0: + return src[start : i + 1] + raise AssertionError(f"unbalanced {opener!r}/{closer!r} after {anchor!r}") + + +def test_main_composer_keydown_rearms_watchdog(): + """After the keydown re-pin sets composingRef=true the watchdog must + be re-armed; otherwise the WSL+Chrome no-compositionend path this PR + targets would lock Send permanently after any IME keypress + (Codex P1 on commit 597af0d0).""" + src = THREAD_TSX.read_text() + block = _extract_block(src, "const onKeyDown = useCallback") + assert "refreshStuckTimer" in block, ( + "main composer keydown gate must call refreshStuckTimer after " + "re-pinning composingRef so the watchdog runs again on the " + "stuck-compositionend path" + ) + assert "clearStuckTimer();" not in block.replace("clearStuckTimer\n", "").replace( + "clearStuckTimer,", "" + ), ( + "main composer keydown gate must not leave the watchdog only " + "cleared — that's the Codex P1 regression" + ) + + +def test_compare_composer_keydown_rearms_watchdog(): + """Same re-arm contract for the compare-mode composer.""" + src = SHARED_TSX.read_text() + block = _extract_block(src, "function onKeyDown", opener = "{", closer = "}") + assert "refreshStuckImeTimer" in block, ( + "compare composer keydown gate must call refreshStuckImeTimer " + "after re-pinning composingRef" + )