From 40420a619b588049f138889add2417cb9dcb7b91 Mon Sep 17 00:00:00 2001 From: Teknium <127238744+teknium1@users.noreply.github.com> Date: Wed, 3 Jun 2026 21:38:58 -0700 Subject: [PATCH] fix(desktop): attachments on Enter, IME composition, scroll, fetchJson resets (salvage #38502) (#38677) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * fix(desktop): critical fixes — attachments, IME composition, scroll, fetchJson DC2: Pass attachments to onSubmit() on direct Enter submit and call clearComposerAttachments(). Previously attachments were silently dropped — only text was sent while attachment pills remained visible. DH1: Add 'open' to ThinkingDisclosure ResizeObserver effect deps. When the disclosure toggles, refs point to new DOM but the observer wasn't reattached, breaking live-scroll preview after expand/collapse and leaking detached DOM nodes. DH3+DH4: Add composition tracking via composingRef (set by compositionstart/compositionend). Guards handleEditorInput (skip preedit state writes), handleEditorKeyDown (prefer composingRef over unreliable isComposing), and form onSubmit (prevent IME Enter from triggering submission). Fixes IME Enter message splitting and preedit text leaking into app state on CJK input. DH6: Add res.on('error', reject) to fetchJson response stream. Without this, a TCP reset mid-transfer left the promise hanging forever, freezing the desktop UI. All TypeScript compiles cleanly. * chore: add copii.list@gmail.com to AUTHOR_MAP (stremtec) * fix(desktop): prevent scroll snap-back during streaming, atomic config writes DH2: Defer pinToBottom() in useLayoutEffect to rAF so that browser scroll/wheel events from the current frame are processed first. Previously an immediate pinToBottom() could snap the viewport back to bottom against the user's trackpad scroll-up intent during streaming — the wheel event hadn't fired yet so stickyBottomRef was still true. DH7: Add writeFileAtomic() helper (write to .tmp then rename) and use it in writeDesktopConnectionConfig, writeDesktopUpdateConfig, and writeBootstrapMarker. Prevents partial writes on crash/power loss that would corrupt JSON config files, requiring manual repair. * fix(desktop): guard nativeTheme listener from duplicates, invalidate connection config cache DM9: Guard nativeTheme.on('updated') with a one-shot flag so that multiple createWindow() calls (e.g. macOS activate after all windows closed) don't accumulate duplicate listeners on the process-wide singleton. DM3: Add mtime-based cache invalidation to readDesktopConnectionConfig. Previously the cache was populated once and never invalidated — if an external tool modified connection.json, the desktop ignored the change until restart. Now re-reads when the file's mtime differs. * fix(desktop): widen fetchJson res.on('error') to sibling fetch + sort JSX props Follow-up to salvaged #38502: - resourceBufferFromUrl had the same mid-stream-reset hang class as fetchJson (req.on('error') present, res.on('error') missing). Add the response-stream error handler so a TCP reset during body read rejects instead of leaving the promise unsettled. - Sort the new onComposition* JSX props to satisfy perfectionist/sort-jsx-props (was an introduced eslint error in the composer). --------- Co-authored-by: asill-livestream --- apps/desktop/electron/main.cjs | 41 +++++++++++++++---- apps/desktop/src/app/chat/composer/index.tsx | 28 +++++++++++-- .../assistant-ui/thread-virtualizer.tsx | 7 +++- .../src/components/assistant-ui/thread.tsx | 4 +- scripts/release.py | 1 + 5 files changed, 68 insertions(+), 13 deletions(-) diff --git a/apps/desktop/electron/main.cjs b/apps/desktop/electron/main.cjs index fb40e4663f..c89b263c59 100644 --- a/apps/desktop/electron/main.cjs +++ b/apps/desktop/electron/main.cjs @@ -471,12 +471,14 @@ let bootstrapAbortController = null // of re-adopting the install we're repairing. Cleared once a bootstrap runs. let forceBootstrapRepair = false let connectionConfigCache = null +let connectionConfigCacheMtime = null const hermesLog = [] const previewWatchers = new Map() let previewShortcutActive = false let desktopLogBuffer = '' let desktopLogFlushTimer = null let desktopLogFlushPromise = Promise.resolve() +let nativeThemeListenerInstalled = false let bootProgressState = { error: null, fakeMode: BOOT_FAKE_MODE, @@ -1101,9 +1103,17 @@ function readDesktopUpdateConfig() { } } +// Atomic file write: temp + rename (atomic on all platforms). Prevents +// partial writes on crash/power loss that corrupt JSON config files. +function writeFileAtomic(targetPath, data, encoding) { + const tmp = targetPath + '.tmp' + fs.writeFileSync(tmp, data, encoding) + fs.renameSync(tmp, targetPath) +} + function writeDesktopUpdateConfig(config) { fs.mkdirSync(path.dirname(DESKTOP_UPDATE_CONFIG_PATH), { recursive: true }) - fs.writeFileSync(DESKTOP_UPDATE_CONFIG_PATH, JSON.stringify(config, null, 2)) + writeFileAtomic(DESKTOP_UPDATE_CONFIG_PATH, JSON.stringify(config, null, 2)) } // Match the backend's source resolution but bias toward a real git checkout. @@ -1628,7 +1638,7 @@ function writeBootstrapMarker(payload) { completedAt: new Date().toISOString(), desktopVersion: app.getVersion() } - fs.writeFileSync(BOOTSTRAP_COMPLETE_MARKER, JSON.stringify(merged, null, 2) + '\n', 'utf8') + writeFileAtomic(BOOTSTRAP_COMPLETE_MARKER, JSON.stringify(merged, null, 2) + '\n', 'utf8') return merged } @@ -2094,6 +2104,7 @@ function fetchJson(url, token, options = {}) { }, res => { const chunks = [] + res.on('error', reject) res.on('data', chunk => chunks.push(chunk)) res.on('end', () => { const text = Buffer.concat(chunks).toString('utf8') @@ -2433,6 +2444,7 @@ async function resourceBufferFromUrl(rawUrl) { return } const chunks = [] + res.on('error', reject) res.on('data', chunk => chunks.push(chunk)) res.on('end', () => { resolve({ @@ -3135,7 +3147,17 @@ function decryptDesktopSecret(secret) { } function readDesktopConnectionConfig() { - if (connectionConfigCache) { + // Check if file changed on disk since last read (e.g. modified by another + // process or an external tool). Our own writes update the cache inline + // via writeDesktopConnectionConfig, but external changes would be missed. + let mtime = null + try { + mtime = fs.statSync(DESKTOP_CONNECTION_CONFIG_PATH).mtimeMs + } catch { + mtime = null + } + + if (connectionConfigCache && connectionConfigCacheMtime === mtime) { return connectionConfigCache } @@ -3156,14 +3178,16 @@ function readDesktopConnectionConfig() { } connectionConfigCache = config + connectionConfigCacheMtime = mtime return config } function writeDesktopConnectionConfig(config) { fs.mkdirSync(path.dirname(DESKTOP_CONNECTION_CONFIG_PATH), { recursive: true }) - fs.writeFileSync(DESKTOP_CONNECTION_CONFIG_PATH, JSON.stringify(config, null, 2)) + writeFileAtomic(DESKTOP_CONNECTION_CONFIG_PATH, JSON.stringify(config, null, 2)) connectionConfigCache = config + connectionConfigCacheMtime = fs.statSync(DESKTOP_CONNECTION_CONFIG_PATH).mtimeMs } function sanitizeDesktopConnectionConfig(config = readDesktopConnectionConfig()) { @@ -3491,9 +3515,12 @@ function createWindow() { } if (!IS_MAC) { - nativeTheme.on('updated', () => { - mainWindow?.setTitleBarOverlay?.(getTitleBarOverlayOptions()) - }) + if (!nativeThemeListenerInstalled) { + nativeThemeListenerInstalled = true + nativeTheme.on('updated', () => { + mainWindow?.setTitleBarOverlay?.(getTitleBarOverlayOptions()) + }) + } } mainWindow.on('will-enter-full-screen', () => sendWindowStateChanged(true)) diff --git a/apps/desktop/src/app/chat/composer/index.tsx b/apps/desktop/src/app/chat/composer/index.tsx index d19c0a2532..49ec676626 100644 --- a/apps/desktop/src/app/chat/composer/index.tsx +++ b/apps/desktop/src/app/chat/composer/index.tsx @@ -142,6 +142,7 @@ export function ChatBar({ const [queueEdit, setQueueEdit] = useState(null) const [focusRequestId, setFocusRequestId] = useState(0) const dragDepthRef = useRef(0) + const composingRef = useRef(false) // true during IME composition (CJK input) const lastSpokenIdRef = useRef(null) const narrow = useMediaQuery('(max-width: 30rem)') @@ -476,6 +477,13 @@ export function ChatBar({ }, [trigger]) const handleEditorInput = (event: FormEvent) => { + // During IME composition the DOM contains uncommitted preedit text + // mixed with real content. Skip state writes — compositionend will + // deliver the finalized text via a clean input event. + if (composingRef.current) { + return + } + const editor = event.currentTarget if (editor.childNodes.length === 1 && editor.firstChild?.nodeName === 'BR') { @@ -576,9 +584,11 @@ export function ChatBar({ const handleEditorKeyDown = (event: KeyboardEvent) => { // IME composition: Enter confirms composed text, not a message submission. - // Without this guard, pressing Enter to finalise a Korean/Japanese/Chinese - // IME preedit fires submitDraft() and splits the message mid-word. - if (event.nativeEvent.isComposing) { + // We check both composingRef (set by compositionstart/compositionend, robust + // across browsers) and nativeEvent.isComposing (Chromium fallback). Without + // this guard, pressing Enter to finalise a Korean/Japanese/Chinese IME + // preedit fires submitDraft() and splits the message mid-word. + if (composingRef.current || event.nativeEvent.isComposing) { return } @@ -971,7 +981,8 @@ export function ChatBar({ const submitted = draft triggerHaptic('submit') clearDraft() - void onSubmit(submitted) + clearComposerAttachments() + void onSubmit(submitted, { attachments }) } focusInput() @@ -1110,6 +1121,12 @@ export function ChatBar({ data-placeholder={placeholder} data-slot={RICH_INPUT_SLOT} onBlur={() => window.setTimeout(closeTrigger, 80)} + onCompositionEnd={() => { + composingRef.current = false + }} + onCompositionStart={() => { + composingRef.current = true + }} onDragOver={handleInputDragOver} onDrop={handleInputDrop} onFocus={() => markActiveComposer('main')} @@ -1158,6 +1175,9 @@ export function ChatBar({ onDrop={handleDrop} onSubmit={e => { e.preventDefault() + if (composingRef.current) { + return + } submitDraft() }} ref={composerRef} diff --git a/apps/desktop/src/components/assistant-ui/thread-virtualizer.tsx b/apps/desktop/src/components/assistant-ui/thread-virtualizer.tsx index 7a5f511070..1a22dac9ee 100644 --- a/apps/desktop/src/components/assistant-ui/thread-virtualizer.tsx +++ b/apps/desktop/src/components/assistant-ui/thread-virtualizer.tsx @@ -430,7 +430,12 @@ function useThreadScrollAnchor({ return } if (groupCount > prevGroupCountForLayoutRef.current && stickyBottomRef.current) { - pinToBottom() + // Defer to rAF so that browser scroll/wheel events from the current + // frame are processed first. Without this deferral, a trackpad + // scroll-up during streaming can race with this effect: the wheel + // event hasn't fired yet so stickyBottomRef is still true, and the + // immediate pinToBottom() would snap the viewport back to bottom + // against the user's intent. requestAnimationFrame(() => { if (stickyBottomRef.current) { pinToBottom() diff --git a/apps/desktop/src/components/assistant-ui/thread.tsx b/apps/desktop/src/components/assistant-ui/thread.tsx index 7cd4a88bb4..fab2c4b7f4 100644 --- a/apps/desktop/src/components/assistant-ui/thread.tsx +++ b/apps/desktop/src/components/assistant-ui/thread.tsx @@ -375,7 +375,9 @@ const ThinkingDisclosure: FC<{ observer.observe(content) return () => observer.disconnect() - }, [isPreview]) + // Re-run when the disclosure toggles so the observer attaches to the new + // DOM after expand/collapse (refs are conditionally rendered on `open`). + }, [isPreview, open]) return (