diff --git a/apps/desktop/src/app/chat/composer/hooks/use-composer-metrics.ts b/apps/desktop/src/app/chat/composer/hooks/use-composer-metrics.ts index 771bf2a9dd39..859e70456c91 100644 --- a/apps/desktop/src/app/chat/composer/hooks/use-composer-metrics.ts +++ b/apps/desktop/src/app/chat/composer/hooks/use-composer-metrics.ts @@ -2,6 +2,7 @@ import { useAuiState } from '@assistant-ui/react' import { type RefObject, useCallback, useEffect, useRef, useState } from 'react' import { + chatSurfaceRoot, clearSurfaceVar, COMPOSER_HEIGHT_VAR, COMPOSER_SURFACE_HEIGHT_VAR, @@ -168,11 +169,16 @@ export function useComposerMetrics({ composerRef, composerSurfaceRef, editorRef, }, [poppedOut, syncComposerMetrics]) useEffect(() => { - const composer = composerRef.current + // Resolve the owning surface while the composer is still attached; the + // unmount cleanup runs after React detached the node, where closest() + // can no longer find [data-chat-surface] and would clear the document + // root instead of this surface (same class of bug as the status stack's + // stale-clearance leak). + const root = chatSurfaceRoot(composerRef.current) return () => { - clearSurfaceVar(composer, COMPOSER_HEIGHT_VAR) - clearSurfaceVar(composer, COMPOSER_SURFACE_HEIGHT_VAR) + clearSurfaceVar(root, COMPOSER_HEIGHT_VAR) + clearSurfaceVar(root, COMPOSER_SURFACE_HEIGHT_VAR) } }, [composerRef]) diff --git a/apps/desktop/src/app/chat/composer/status-stack/index.tsx b/apps/desktop/src/app/chat/composer/status-stack/index.tsx index fbbd2b94d5aa..41c84bfdaace 100644 --- a/apps/desktop/src/app/chat/composer/status-stack/index.tsx +++ b/apps/desktop/src/app/chat/composer/status-stack/index.tsx @@ -3,7 +3,7 @@ import { type ReactNode, useEffect, useLayoutEffect, useMemo, useRef } from 'rea import { useNavigate } from 'react-router-dom' import { blurComposerInput } from '@/app/chat/composer/focus' -import { clearSurfaceVar, setSurfaceVar, STATUS_STACK_VAR } from '@/app/chat/surface-vars' +import { chatSurfaceRoot, clearSurfaceVar, setSurfaceVar, STATUS_STACK_VAR } from '@/app/chat/surface-vars' import { AGENTS_ROUTE } from '@/app/routes' import { BillingBanner } from '@/components/billing-banner' import { composerDockCard } from '@/components/chat/composer-dock' @@ -220,11 +220,16 @@ export function ComposerStatusStack({ queue, sessionId }: ComposerStatusStackPro const el = stackRef.current if (!visible || !el) { - clearSurfaceVar(el, STATUS_STACK_VAR) - return } + // Resolve the owning surface NOW, while the node is attached. The cleanup + // below runs after the stack collapsed and React removed the div, so + // closest() from the detached node misses [data-chat-surface] and would + // clear the document root instead — leaving the stale height on the + // surface, which keeps inflating the thread's bottom clearance until the + // next publish. + const root = chatSurfaceRoot(el) let last = -1 const sync = () => { @@ -242,7 +247,7 @@ export function ComposerStatusStack({ queue, sessionId }: ComposerStatusStackPro return () => { observer.disconnect() - clearSurfaceVar(el, STATUS_STACK_VAR) + clearSurfaceVar(root, STATUS_STACK_VAR) } }, [visible]) diff --git a/apps/desktop/src/app/chat/composer/status-stack/surface-var-lifecycle.test.tsx b/apps/desktop/src/app/chat/composer/status-stack/surface-var-lifecycle.test.tsx new file mode 100644 index 000000000000..a01563aedc34 --- /dev/null +++ b/apps/desktop/src/app/chat/composer/status-stack/surface-var-lifecycle.test.tsx @@ -0,0 +1,94 @@ +import { act, cleanup, render } from '@testing-library/react' +import { MemoryRouter } from 'react-router-dom' +import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest' + +import { STATUS_STACK_VAR } from '@/app/chat/surface-vars' +import { I18nProvider } from '@/i18n' +import { $goalsBySession, type SessionGoal } from '@/store/goals' + +import { ComposerStatusStack } from './index' + +// The stack measures itself into a surface var — jsdom has no ResizeObserver. +class ResizeObserverStub { + observe() {} + unobserve() {} + disconnect() {} +} + +vi.stubGlobal('ResizeObserver', ResizeObserverStub) + +const SID = 'sess-height-1' + +const goal = (): SessionGoal => ({ status: 'active', title: 'ship the feature', updatedAt: Date.now() }) + +/** + * Regression: when the stack collapses (its last item finishes), React removes + * the stack div BEFORE the layout-effect cleanup runs. Resolving the surface + * from the ref at cleanup time then walks a DETACHED node, misses + * [data-chat-surface], and clears the document root instead — the stale height + * stays on the surface and keeps inflating the thread's bottom clearance + * (`--thread-last-message-clearance`) until the next publish. The effect must + * capture its surface root while the node is still attached. + */ +describe('ComposerStatusStack surface-var lifecycle', () => { + beforeEach(() => { + $goalsBySession.set({}) + }) + + afterEach(() => { + cleanup() + $goalsBySession.set({}) + document.documentElement.style.removeProperty(STATUS_STACK_VAR) + }) + + function renderOnSurface() { + const surface = document.createElement('div') + surface.setAttribute('data-chat-surface', '') + document.body.append(surface) + + const view = render( + + + + + , + { container: surface } + ) + + return { surface, view } + } + + it('publishes its measured height onto the owning surface while visible', () => { + $goalsBySession.set({ [SID]: goal() }) + + const { surface } = renderOnSurface() + + // jsdom measures 0 — the value is irrelevant, the target element is not. + expect(surface.style.getPropertyValue(STATUS_STACK_VAR)).toBe('0px') + expect(document.documentElement.style.getPropertyValue(STATUS_STACK_VAR)).toBe('') + }) + + it('clears the surface var when the stack collapses to nothing', () => { + $goalsBySession.set({ [SID]: goal() }) + + const { surface } = renderOnSurface() + expect(surface.style.getPropertyValue(STATUS_STACK_VAR)).toBe('0px') + + // Last status item goes away → the component renders null and React + // detaches the stack div before the cleanup runs. + act(() => $goalsBySession.set({})) + + expect(surface.style.getPropertyValue(STATUS_STACK_VAR)).toBe('') + }) + + it('clears the surface var on unmount', () => { + $goalsBySession.set({ [SID]: goal() }) + + const { surface, view } = renderOnSurface() + expect(surface.style.getPropertyValue(STATUS_STACK_VAR)).toBe('0px') + + view.unmount() + + expect(surface.style.getPropertyValue(STATUS_STACK_VAR)).toBe('') + }) +})