diff --git a/src/browser/components/AppLoader/AppLoader.tsx b/src/browser/components/AppLoader/AppLoader.tsx index 507499c283..a5deec8631 100644 --- a/src/browser/components/AppLoader/AppLoader.tsx +++ b/src/browser/components/AppLoader/AppLoader.tsx @@ -11,6 +11,7 @@ import { useGitStatusStoreRaw } from "../../stores/GitStatusStore"; import { useRuntimeStatusStoreRaw } from "../../stores/RuntimeStatusStore"; import { useBackgroundBashStoreRaw } from "../../stores/BackgroundBashStore"; import { getPRStatusStoreInstance } from "../../stores/PRStatusStore"; +import { getProvidersConfigStore } from "../../stores/ProvidersConfigStore"; import { ProjectProvider, useProjectContext } from "../../contexts/ProjectContext"; import { PolicyProvider, usePolicy } from "@/browser/contexts/PolicyContext"; import { PolicyBlockedScreen } from "@/browser/components/PolicyBlockedScreen/PolicyBlockedScreen"; @@ -182,6 +183,7 @@ function AppLoaderInner() { runtimeStatusStore.setClient(api ?? null); backgroundBashStore.setClient(api ?? null); getPRStatusStoreInstance().setClient(api ?? null); + getProvidersConfigStore().setClient(api ?? null); if (!workspaceContext.loading) { workspaceStoreInstance.syncWorkspaces(workspaceContext.workspaceMetadata); diff --git a/src/browser/components/ArchivedWorkspaces/ArchivedWorkspaces.stories.tsx b/src/browser/components/ArchivedWorkspaces/ArchivedWorkspaces.stories.tsx index e96013dace..d34965628a 100644 --- a/src/browser/components/ArchivedWorkspaces/ArchivedWorkspaces.stories.tsx +++ b/src/browser/components/ArchivedWorkspaces/ArchivedWorkspaces.stories.tsx @@ -10,6 +10,7 @@ import { createMockORPCClient } from "@/browser/stories/mocks/orpc"; import { NOW, createArchivedWorkspace } from "@/browser/stories/mocks/workspaces"; import { lightweightMeta } from "@/browser/stories/meta.js"; import { useWorkspaceStoreRaw } from "@/browser/stores/WorkspaceStore"; +import { getProvidersConfigStore } from "@/browser/stores/ProvidersConfigStore"; import { getArchivedWorkspacesExpandedKey } from "@/common/constants/storage"; import type { FrontendWorkspaceMetadata } from "@/common/types/workspace"; import { ArchivedWorkspaces } from "./ArchivedWorkspaces"; @@ -73,8 +74,12 @@ function ArchivedWorkspacesStoryShell(props: { useEffect(() => { workspaceStore.setClient(client); + // useProvidersConfig consumers read the shared store, which gets its + // client from AppLoader in the real app — wire it manually here too. + getProvidersConfigStore().setClient(client); return () => { workspaceStore.setClient(null); + getProvidersConfigStore().setClient(null); }; }, [client, workspaceStore]); diff --git a/src/browser/components/ChatPane/ChatPane.tsx b/src/browser/components/ChatPane/ChatPane.tsx index 0fe05f7f6e..38f1b0739d 100644 --- a/src/browser/components/ChatPane/ChatPane.tsx +++ b/src/browser/components/ChatPane/ChatPane.tsx @@ -23,6 +23,7 @@ import { StreamingBarrier } from "@/browser/features/Messages/ChatBarrier/Stream import { RetryBarrier } from "@/browser/features/Messages/ChatBarrier/RetryBarrier"; import { PinnedTodoList } from "../PinnedTodoList/PinnedTodoList"; import { ChatInputDecorationStackLane, TranscriptTailStackLane } from "./LayoutStackLane"; +import { computeChatViewReveal, useChatViewDataReady } from "./useChatViewDataReady"; import { TranscriptHydrationSkeleton } from "./TranscriptHydrationSkeleton"; import { createChatInputDecorationStackItem, @@ -200,19 +201,14 @@ type ReviewsState = ReturnType; const TRANSCRIPT_CONTENT_NO_ANCHOR_STYLE = { overflowAnchor: "none" } as const; // The sentinel is the sole anchor candidate while locked. const TRANSCRIPT_BOTTOM_SENTINEL_STYLE = { overflowAnchor: "auto" } as const; -// The composer floats over the bottom of the transcript region so its height -// changes never resize the scrollport's clientHeight (the root cause of the -// send-time "viewport resize from below" flash). The scrollport instead reserves -// clearance via bottom padding equal to the live composer height (--composer-h, -// published by a ResizeObserver in ChatPaneContent); 15px matches the original -// uniform p-[15px] inset. -const TRANSCRIPT_SCROLLPORT_STYLE: React.CSSProperties = { - paddingBottom: "calc(15px + var(--composer-h, 0px))", -}; -// Keep the jump-to-bottom chip just above the floating composer (8px gap). -const JUMP_TO_BOTTOM_CHIP_STYLE: React.CSSProperties = { - bottom: "calc(var(--composer-h, 0px) + 8px)", -}; +// The composer dock is normal scroll content (sticky to the scrollport bottom), +// so the transcript's bottom clearance is reserved by flow layout in the SAME +// layout pass a decoration/textarea height change happens — there is no +// measured channel (the old --composer-h ResizeObserver) that could lag actual +// layout by a frame and tear. The dock must never be a scroll-anchoring +// candidate: while locked the sentinel owns anchoring, and while released the +// browser must anchor to a transcript row, not the sticky dock. +const COMPOSER_DOCK_STYLE = { overflowAnchor: "none" } as const; function findTranscriptMessageElement( scrollContainer: HTMLElement, @@ -255,10 +251,6 @@ export const ChatPane: React.FC = (props) => { ref={chatAreaRef} aria-hidden={immersiveHidden || undefined} className={cn( - // `relative` is the positioning context for the floating composer dock - // (absolute bottom of this column); it overlays the bottom of the flex-1 - // transcript region without taking flex space, so composer height changes - // never resize the transcript scrollport. "bg-surface-primary relative flex min-w-96 flex-1 flex-col", // Immersive review overlays the entire workspace, so hiding the chat pane removes // its layout cost while preserving component state for the return transition. @@ -347,6 +339,11 @@ const ChatPaneContent: React.FC = (props) => { const { config: providersConfig } = useProvidersConfig(); + // First-paint readiness barrier: all decoration data sources known (or the + // resilience deadline passed). Gates the reveal so decorations can't pop in + // after the transcript is visible. + const chatViewDataReady = useChatViewDataReady(workspaceId); + const { threshold: autoCompactionThreshold } = useAutoCompactionSettings( workspaceId, pendingModel @@ -550,7 +547,6 @@ const ChatPaneContent: React.FC = (props) => { autoScroll, disableAutoScroll, jumpToBottom, - reanchorBottom, handleScroll, markUserScrollIntent, handleScrollContainerWheel, @@ -560,30 +556,17 @@ const ChatPaneContent: React.FC = (props) => { handleScrollContainerKeyDown, } = useAutoScroll(); - // The composer floats over the transcript region (see TRANSCRIPT_SCROLLPORT_STYLE). - // Publish its measured height as `--composer-h` on the transcript region so the - // scrollport can reserve matching bottom clearance, and re-pin when it grows - // (composer growth only changes the scrollport's bottom padding, which neither - // native anchoring nor the content ResizeObserver can see — see reanchorBottom). - const transcriptRegionRef = useRef(null); - const composerRef = useRef(null); - useLayoutEffect(() => { - const region = transcriptRegionRef.current; - const composer = composerRef.current; - const ResizeObserverCtor = typeof window !== "undefined" ? window.ResizeObserver : undefined; - if (!region || !composer || !ResizeObserverCtor) return; - - const applyComposerHeight = () => { - const heightPx = Math.ceil(composer.getBoundingClientRect().height); - region.style.setProperty("--composer-h", `${heightPx}px`); - reanchorBottom(); - }; - - applyComposerHeight(); - const observer = new ResizeObserverCtor(applyComposerHeight); - observer.observe(composer); - return () => observer.disconnect(); - }, [reanchorBottom]); + // The composer dock lives inside the scrollport (sticky to its bottom), so + // mousedown/keydown events from the composer bubble to the transcript + // handlers. They must not open a scroll-intent window or clear the + // side-question hold: typing or clicking in the composer is not transcript + // scroll intent. Wheel/touch are intentionally NOT filtered — those gestures + // really do scroll the transcript (native scroll chaining), so they must keep + // marking user intent or the bottom lock would fight the user's scroll. + const composerDockRef = useRef(null); + const isComposerDockEvent = useCallback((target: EventTarget | null): boolean => { + return target instanceof Node && (composerDockRef.current?.contains(target) ?? false); + }, []); const sideQuestionScrollHoldRef = useRef({ initialized: false, @@ -698,10 +681,13 @@ const ChatPaneContent: React.FC = (props) => { const handleTranscriptMouseDown = useCallback( (event: React.MouseEvent) => { + if (isComposerDockEvent(event.target)) { + return; + } clearActiveSideQuestionScrollHold(); handleScrollContainerMouseDown(event); }, - [clearActiveSideQuestionScrollHold, handleScrollContainerMouseDown] + [clearActiveSideQuestionScrollHold, handleScrollContainerMouseDown, isComposerDockEvent] ); const handleTranscriptTouchMove = useCallback(() => { @@ -711,10 +697,13 @@ const ChatPaneContent: React.FC = (props) => { const handleTranscriptKeyDown = useCallback( (event: React.KeyboardEvent) => { + if (isComposerDockEvent(event.target)) { + return; + } clearActiveSideQuestionScrollHold(); handleScrollContainerKeyDown(event); }, - [clearActiveSideQuestionScrollHold, handleScrollContainerKeyDown] + [clearActiveSideQuestionScrollHold, handleScrollContainerKeyDown, isComposerDockEvent] ); const handleJumpToBottom = useCallback(() => { @@ -989,9 +978,16 @@ const ChatPaneContent: React.FC = (props) => { const shouldShowStreamingBarrier = isStreamStarting || canInterrupt; // Keep rendering cached transcript rows during incremental catch-up so workspace switches // feel stable, but active stream-start/interrupt states should keep their barrier visible - // instead of flashing full-height transcript placeholders. - const showTranscriptHydrationPlaceholder = - isHydratingTranscript && deferredMessages.length === 0 && !shouldShowStreamingBarrier; + // instead of flashing full-height transcript placeholders. The skeleton additionally holds + // until decoration data sources are known so the transcript and all composer decorations + // reveal in ONE commit — see useChatViewDataReady for the contract. + const { showHydrationPlaceholder: showTranscriptHydrationPlaceholder, revealDecorations } = + computeChatViewReveal({ + isHydratingTranscript, + chatViewDataReady, + hasRenderableMessages: deferredMessages.length > 0, + shouldShowStreamingBarrier, + }); const showEmptyTranscriptPlaceholder = deferredMessages.length === 0 && !showTranscriptHydrationPlaceholder && @@ -1235,13 +1231,10 @@ const ChatPaneContent: React.FC = (props) => { <> {/* Spacer for fixed mobile header - mobile-header-spacer adds padding-top on touch devices. - `flex-1` keeps this region filling all space below the header regardless of the floating - composer (which is out of flow), so the scrollport's clientHeight never changes when the - composer resizes. `--composer-h` is published here for the scrollport clearance + jump chip. */} -
+ The composer dock is IN-FLOW scroll content (sticky to the scrollport + bottom), so this region — and therefore the scrollport's clientHeight — + never resizes when the composer grows or shrinks. */} +
= (props) => { onKeyDown={handleTranscriptKeyDown} onScroll={handleScroll} onContextMenu={transcriptContextMenu.onContextMenu} - role="log" - aria-live={canInterrupt ? "polite" : "off"} - aria-busy={canInterrupt || isHydratingTranscript} - aria-label="Conversation transcript" tabIndex={0} data-testid="message-window" - data-loaded={!loading && !isHydratingTranscript} + // Settled marker for perf tests and story play helpers: includes + // decoration data readiness so waiting on it observes the chat + // view's final (post-reveal) layout. + data-loaded={!loading && !isHydratingTranscript && chatViewDataReady} // Browser scroll anchoring stays ENABLED on the scrollport; the // overflow-anchor policy lives on the inner content (opt rows out while - // locked so the bottom sentinel is the sole anchor). `paddingBottom` - // reserves clearance for the floating composer (--composer-h) so the - // last message scrolls clear of it instead of resizing the viewport. - style={TRANSCRIPT_SCROLLPORT_STYLE} + // locked so the bottom sentinel is the sole anchor). No bottom padding: + // clearance for the composer is the in-flow dock itself, so it is always + // exact and reserved in the same layout pass as any dock height change. + // The flex column makes the transcript content stretch (flex-1) so the + // dock sits at the scrollport bottom even when the transcript is short. // The named `transcript` container is what the sticky plan TOC queries // for visibility — using a container query rather than a viewport media // query means sidebars opening/closing correctly hide the TOC even when // the viewport width is unchanged. See `.plan-toc-aside` in globals.css. - className="@container/transcript h-full overflow-x-hidden overflow-y-auto px-[15px] pt-[15px] leading-[1.5] break-words whitespace-pre-wrap" + className="@container/transcript flex h-full flex-col overflow-x-hidden overflow-y-auto px-[15px] pt-[15px] leading-[1.5] break-words whitespace-pre-wrap" >
{showTranscriptHydrationPlaceholder ? ( @@ -1474,18 +1478,16 @@ const ChatPaneContent: React.FC = (props) => { )} - +
{/* Bottom anchor: a 0-height sibling of the transcript content. While locked it is the sole `overflow-anchor: auto` element, so native CSS scroll anchoring keeps it (and therefore the bottom) pinned as rows - append above it. It sits below the inner content but above the - scrollport's composer-clearance padding, so the last message clears - the floating composer. */} + append above it. It sits between the transcript content and the + in-flow composer dock: appends grow content ABOVE it (anchoring + compensates), while dock growth happens BELOW it (covered by the + scrollport-children ResizeObserver in useAutoScroll, which re-pins + before paint). */}
= (props) => { className="h-0 w-full" style={TRANSCRIPT_BOTTOM_SENTINEL_STYLE} /> + + {/* The composer dock is in-flow scroll content stuck to the scrollport + bottom: clearance for the last message is reserved by normal flow + layout in the same pass as any dock height change (no measured + --composer-h channel to lag a frame behind and tear), while + `sticky` keeps the composer visually pinned over the transcript + when the user scrolls up. `mx-[-15px]` cancels the scrollport's + horizontal padding so the dock stays full-bleed; `whitespace-normal + break-normal` reset the transcript text inheritance. clientHeight + of the scrollport never changes with dock height (send-flash + invariant). `bg-surface-primary` keeps transcript content from + showing through gaps between decoration banners. */} +
+ {!autoScroll && ( + + )} + {transcriptOnly ? ( + // Transcript-only workspaces keep their historical transcript, but the whole + // composer surface is replaced with a single read-only notice. + + ) : ( + void handleEditQueuedMessage()} + onSendQueuedImmediately={ + workspaceState?.canInterrupt ? handleSendQueuedImmediately : undefined + } + reviews={reviews} + onCheckReviews={handleCheckReviews} + /> + )} +
+
{transcriptContextMenu.menu} - {!autoScroll && ( - - )} -
- - - {/* The composer floats over the bottom of the chat column instead of taking - flex space, so its height changes never resize the transcript scrollport - (the send-time "viewport resize from below" flash). `bg-surface-primary` - keeps transcript content from showing through gaps between decoration - banners. `composerRef` feeds the scrollport clearance (--composer-h). */} -
- {transcriptOnly ? ( - // Transcript-only workspaces keep their historical transcript, but the whole - // composer surface is replaced with a single read-only notice. - - ) : ( - void handleEditQueuedMessage()} - onSendQueuedImmediately={ - workspaceState?.canInterrupt ? handleSendQueuedImmediately : undefined - } - reviews={reviews} - onCheckReviews={handleCheckReviews} - /> - )}
@@ -1587,13 +1598,18 @@ interface ChatInputPaneProps { workspaceId: string; projectName: string; workspaceName: string; + /** + * False until the chat view's one-commit reveal (transcript + decorations + * together). The decoration lane stays empty before that so a decoration + * can never mount after paint and shift the transcript. + */ + revealDecorations: boolean; runtimeConfig?: RuntimeConfig; isPreStreamAgentTask: boolean; preStreamAgentTaskStatus: "queued" | "starting"; isCompacting: boolean; isStreamStarting: boolean; isTranscriptCaughtUp: boolean; - isHydratingTranscript: boolean; shouldShowPinnedTodoList: boolean; shouldShowReviewsBanner: boolean; concurrentLocalStreamingWorkspaceName: string | null; @@ -1716,17 +1732,17 @@ const ChatInputPane: React.FC = (props) => { ), }); } - // The decoration lane lives inside the floating composer dock, so its height is - // captured by the composer ResizeObserver (--composer-h) and never resizes the - // transcript scrollport; the bottom stays pinned via native anchoring + reanchorBottom. + // The decoration lane lives inside the in-flow sticky composer dock, so a + // decoration mounting/unmounting reflows the transcript clearance in the same + // layout pass; the bottom stays pinned via native anchoring plus the + // scrollport-children ResizeObserver in useAutoScroll. Until the one-commit + // reveal the lane renders empty: readiness is monotonic per mounted + // workspace, so this only ever delays the initial mount — it never unmounts + // visible decorations. return ( <> - + void) | null = null; -let originalResizeObserver: typeof ResizeObserver | undefined; -const resizeCallbacks = new Map(); const COMPOSER_STACK_COMPONENT = "ChatInputDecorationStack"; const TRANSCRIPT_TAIL_STACK_COMPONENT = "TranscriptTailStack"; -class ResizeObserverMock implements ResizeObserver { - public readonly callback: ResizeObserverCallback; - - constructor(callback: ResizeObserverCallback) { - this.callback = callback; - } - - observe(target: Element) { - resizeCallbacks.set(target, [...(resizeCallbacks.get(target) ?? []), this.callback]); - } - - unobserve(target: Element) { - const callbacks = (resizeCallbacks.get(target) ?? []).filter( - (callback) => callback !== this.callback - ); - if (callbacks.length === 0) { - resizeCallbacks.delete(target); - return; - } - resizeCallbacks.set(target, callbacks); - } - - disconnect() { - for (const [target, callbacks] of resizeCallbacks.entries()) { - const remainingCallbacks = callbacks.filter((callback) => callback !== this.callback); - if (remainingCallbacks.length === 0) { - resizeCallbacks.delete(target); - continue; - } - resizeCallbacks.set(target, remainingCallbacks); - } - } - - takeRecords(): ResizeObserverEntry[] { - return []; - } -} - -function emitResize(target: Element, height: number) { - const callbacks = resizeCallbacks.get(target) ?? []; - const contentRect: DOMRectReadOnly = { - x: 0, - y: 0, - width: 0, - height, - top: 0, - right: 0, - bottom: height, - left: 0, - toJSON: () => ({}), - }; - const entry: ResizeObserverEntry = { - target, - contentRect, - borderBoxSize: [], - contentBoxSize: [], - devicePixelContentBoxSize: [], - }; - for (const callback of callbacks) { - callback([entry], {} as ResizeObserver); - } -} - function getRenderedStack(container: HTMLElement, dataComponent: string): HTMLDivElement { const stack = container.querySelector(`[data-component="${dataComponent}"]`); expect(stack).toBeTruthy(); @@ -88,260 +18,58 @@ function getRenderedStack(container: HTMLElement, dataComponent: string): HTMLDi return stack as HTMLDivElement; } -function getStackContent(container: HTMLElement, dataComponent: string): HTMLDivElement { - const content = getRenderedStack(container, dataComponent).firstElementChild; - expect(content).toBeTruthy(); - if (content?.tagName !== "DIV") { - throw new Error("Expected stack content to exist"); - } - return content as HTMLDivElement; -} - -async function waitForResizeObservation(target: Element): Promise { - await waitFor(() => { - const callbacks = resizeCallbacks.get(target); - if (!callbacks || callbacks.length === 0) { - throw new Error("Resize observer is not attached yet"); - } - }); -} - -function createTextItem(key: string, text: string): ChatInputDecorationStackItem { - return createChatInputDecorationStackItem({ key, node:
{text}
}); -} - -function createHiddenItem(key = "idle-decoration"): ChatInputDecorationStackItem { - return createChatInputDecorationStackItem({ key, node:
}), + createChatInputDecorationStackItem({ key: "second", node:
second banner
}), + ]} /> ); - const hydratingContent = getStackContent(view.container, COMPOSER_STACK_COMPONENT); - await waitForResizeObservation(hydratingContent); - emitResize(hydratingContent, 0); - - await waitFor(() => { - expect(getRenderedStack(view.container, COMPOSER_STACK_COMPONENT).style.minHeight).toBe( - "184px" - ); - }); - - view.rerender( - - ); - - await waitFor(() => { - expect(getRenderedStack(view.container, COMPOSER_STACK_COMPONENT).style.minHeight).toBe(""); - }); - - view.rerender( - - ); - - await waitFor(() => { - expect(getRenderedStack(view.container, COMPOSER_STACK_COMPONENT).style.minHeight).toBe(""); - }); - }); - - it("attaches ResizeObserver when items mount after an empty null lane", async () => { - const view = render( - - ); - - expect( - view.container.querySelector(`[data-component="${COMPOSER_STACK_COMPONENT}"]`) - ).toBeNull(); - - view.rerender( - - ); - - const mountedContent = getStackContent(view.container, COMPOSER_STACK_COMPONENT); - await waitForResizeObservation(mountedContent); - emitResize(mountedContent, 123); - - view.rerender( - - ); - - await waitFor(() => { - expect(getRenderedStack(view.container, COMPOSER_STACK_COMPONENT).style.minHeight).toBe( - "123px" - ); - }); - }); - - it("clears settled empty-lane measurements from both the workspace cache and fallback", async () => { - const view = render( - - ); - - const initialContent = getStackContent(view.container, COMPOSER_STACK_COMPONENT); - await waitForResizeObservation(initialContent); - emitResize(initialContent, 184); - - view.rerender( - - ); - - const settledEmptyContent = getStackContent(view.container, COMPOSER_STACK_COMPONENT); - await waitForResizeObservation(settledEmptyContent); - emitResize(settledEmptyContent, 0); - - view.rerender( - - ); - - await waitFor(() => { - expect(getRenderedStack(view.container, COMPOSER_STACK_COMPONENT).style.minHeight).toBe(""); - }); - - view.rerender( - - ); - - await waitFor(() => { - expect(getRenderedStack(view.container, COMPOSER_STACK_COMPONENT).style.minHeight).toBe(""); - }); + const stack = getRenderedStack(view.container, COMPOSER_STACK_COMPONENT); + expect(stack.textContent).toBe("first bannersecond banner"); }); - it("renders semantic lane policies correctly", () => { + it("opts the transcript tail out of scroll anchoring but not the composer decorations", () => { + // The tail lane renders inside the scrollport above the bottom sentinel, so it + // must never be an anchor candidate while the transcript is locked; composer + // decorations live in the sticky dock, which opts out as a whole in ChatPane. const view = render( -
+ <> + barrier
})]} + /> banner
})]} /> -
Input
- + ); - const decoration = getRenderedStack(view.container, COMPOSER_STACK_COMPONENT); - expect(decoration.className).toContain("justify-end"); - expect(decoration.style.overflowAnchor).toBe(""); - - const tail = render( - + expect( + getRenderedStack(view.container, TRANSCRIPT_TAIL_STACK_COMPONENT).style.overflowAnchor + ).toBe("none"); + expect(getRenderedStack(view.container, COMPOSER_STACK_COMPONENT).style.overflowAnchor).toBe( + "" ); - const tailStack = getRenderedStack(tail.container, TRANSCRIPT_TAIL_STACK_COMPONENT); - expect(tailStack.className).toContain("justify-start"); - expect(tailStack.style.overflowAnchor).toBe("none"); }); }); diff --git a/src/browser/components/ChatPane/LayoutStackLane.tsx b/src/browser/components/ChatPane/LayoutStackLane.tsx index bd800cb432..57f0947b69 100644 --- a/src/browser/components/ChatPane/LayoutStackLane.tsx +++ b/src/browser/components/ChatPane/LayoutStackLane.tsx @@ -1,42 +1,32 @@ -import React, { useLayoutEffect, useRef } from "react"; -import { - clearLayoutStackHeight, - getReservedLayoutStackHeightPx, - measureLayoutStackHeightPx, - rememberLayoutStackHeight, - type ChatInputDecorationStackItem, - type LayoutStackLaneKind, - type TranscriptTailStackItem, +import React from "react"; +import type { + ChatInputDecorationStackItem, + LayoutStackLaneKind, + TranscriptTailStackItem, } from "./layoutStack"; interface LayoutStackLaneConfig { - align: "start" | "end"; dataComponent: string; overflowAnchor?: "none"; } const LAYOUT_STACK_LANE_CONFIG: Record = { "transcript-tail": { - align: "start", dataComponent: "TranscriptTailStack", overflowAnchor: "none", }, "composer-decoration": { - align: "end", dataComponent: "ChatInputDecorationStack", }, }; -interface BaseLayoutStackLaneProps { - workspaceId: string; - isHydrating: boolean; -} +const NO_ANCHOR_STYLE: React.CSSProperties = { overflowAnchor: "none" }; -interface TranscriptTailStackLaneProps extends BaseLayoutStackLaneProps { +interface TranscriptTailStackLaneProps { items: readonly TranscriptTailStackItem[]; } -interface ChatInputDecorationStackLaneProps extends BaseLayoutStackLaneProps { +interface ChatInputDecorationStackLaneProps { items: readonly ChatInputDecorationStackItem[]; } @@ -50,120 +40,30 @@ type LayoutStackLaneProps = * * Lane semantics are intentionally centralized here: * - transcript tail: content that belongs in the scrollport after messages and - * must opt out of browser scroll anchoring. - * - composer decoration: persistent workspace chrome above the textarea whose - * height changes are handled by the transcript scroll owner from outside the - * scrollport. + * must opt out of browser scroll anchoring (so the bottom sentinel stays the + * sole anchor while the transcript is locked to the bottom). + * - composer decoration: persistent workspace chrome above the textarea, inside + * the in-flow sticky composer dock. Because the dock is normal scroll content, + * a decoration mounting/unmounting reflows the transcript clearance in the + * same layout pass — no height measurement or reservation is needed. * * This keeps future warnings/banners from accidentally reintroducing the class of * flash where appending a message moves a live tail row before bottom-lock settles. */ const LayoutStackLane: React.FC = (props) => { - const contentRef = useRef(null); - const stackHeightByWorkspaceIdRef = useRef(new Map()); - const lastMeasuredStackHeightRef = useRef(0); - const laneConfig = LAYOUT_STACK_LANE_CONFIG[props.lane]; - - const hasItems = props.items.length > 0; - const reservedStackHeightPx = getReservedLayoutStackHeightPx({ - workspaceId: props.workspaceId, - isHydrating: props.isHydrating, - stackHeightByWorkspaceId: stackHeightByWorkspaceIdRef.current, - fallbackStackHeightPx: lastMeasuredStackHeightRef.current, - }); - - useLayoutEffect(() => { - const content = contentRef.current; - if (!content) { - return; - } - - const observer = new ResizeObserver((entries) => { - const nextHeight = measureLayoutStackHeightPx(content, entries[0]?.contentRect.height); - if (nextHeight === 0) { - // Some owners (e.g. background-process dialogs) stay mounted while - // rendering nothing. Only drop the reservation after hydration ends — - // transient zero-height observations during hydration must not clobber - // the remembered real height. - if (!props.isHydrating) { - clearLayoutStackHeight( - props.workspaceId, - stackHeightByWorkspaceIdRef.current, - lastMeasuredStackHeightRef - ); - } - } else { - rememberLayoutStackHeight( - props.workspaceId, - nextHeight, - stackHeightByWorkspaceIdRef.current, - lastMeasuredStackHeightRef - ); - } - }); - - observer.observe(content); - return () => { - observer.disconnect(); - }; - }, [hasItems, props.isHydrating, props.workspaceId]); - - // Post-hydration settle: once we're no longer hydrating and have no items, clear - // any cached height so the next hydration doesn't reserve stale space. - useLayoutEffect(() => { - if (props.isHydrating) { - return; - } - - if (!hasItems) { - clearLayoutStackHeight( - props.workspaceId, - stackHeightByWorkspaceIdRef.current, - lastMeasuredStackHeightRef - ); - return; - } - - const content = contentRef.current; - if (!content) { - return; - } - - const settledHeightPx = measureLayoutStackHeightPx(content); - if (settledHeightPx === 0) { - clearLayoutStackHeight( - props.workspaceId, - stackHeightByWorkspaceIdRef.current, - lastMeasuredStackHeightRef - ); - } - }, [hasItems, props.isHydrating, props.workspaceId]); - - if (!hasItems && reservedStackHeightPx === null) { + if (props.items.length === 0) { return null; } - const style: React.CSSProperties = {}; - if (reservedStackHeightPx !== null) { - style.minHeight = `${reservedStackHeightPx}px`; - } - if (laneConfig.overflowAnchor === "none") { - style.overflowAnchor = "none"; - } - + const laneConfig = LAYOUT_STACK_LANE_CONFIG[props.lane]; return (
-
- {props.items.map((item) => ( - {item.node} - ))} -
+ {props.items.map((item) => ( + {item.node} + ))}
); }; diff --git a/src/browser/components/ChatPane/layoutStack.ts b/src/browser/components/ChatPane/layoutStack.ts index 8cd2c7f798..81f027adf8 100644 --- a/src/browser/components/ChatPane/layoutStack.ts +++ b/src/browser/components/ChatPane/layoutStack.ts @@ -1,4 +1,4 @@ -import type { MutableRefObject, ReactNode } from "react"; +import type { ReactNode } from "react"; export type LayoutStackLaneKind = "transcript-tail" | "composer-decoration"; @@ -36,48 +36,3 @@ export function createChatInputDecorationStackItem( ): ChatInputDecorationStackItem { return createLayoutStackItem("composer-decoration", item); } - -interface ReservedLayoutStackHeightProps { - workspaceId: string; - isHydrating: boolean; - stackHeightByWorkspaceId: Map; - fallbackStackHeightPx: number; -} - -export function getReservedLayoutStackHeightPx( - props: ReservedLayoutStackHeightProps -): number | null { - if (!props.isHydrating) { - return null; - } - - const reservedStackHeight = - props.stackHeightByWorkspaceId.get(props.workspaceId) ?? props.fallbackStackHeightPx; - return reservedStackHeight > 0 ? reservedStackHeight : null; -} - -export function measureLayoutStackHeightPx( - content: HTMLElement, - observedHeightPx?: number | null -): number { - return Math.max(0, Math.round(observedHeightPx ?? content.getBoundingClientRect().height)); -} - -export function rememberLayoutStackHeight( - workspaceId: string, - heightPx: number, - stackHeightByWorkspaceId: Map, - lastMeasuredStackHeightRef: MutableRefObject -): void { - lastMeasuredStackHeightRef.current = heightPx; - stackHeightByWorkspaceId.set(workspaceId, heightPx); -} - -export function clearLayoutStackHeight( - workspaceId: string, - stackHeightByWorkspaceId: Map, - lastMeasuredStackHeightRef: MutableRefObject -): void { - lastMeasuredStackHeightRef.current = 0; - stackHeightByWorkspaceId.set(workspaceId, 0); -} diff --git a/src/browser/components/ChatPane/useChatViewDataReady.test.ts b/src/browser/components/ChatPane/useChatViewDataReady.test.ts new file mode 100644 index 0000000000..e7ce31758b --- /dev/null +++ b/src/browser/components/ChatPane/useChatViewDataReady.test.ts @@ -0,0 +1,80 @@ +import { describe, expect, test } from "bun:test"; +import { computeChatViewReveal } from "./useChatViewDataReady"; + +// These cover the reveal *decision* (the branching that makes the chat view +// mount transcript + decorations in one commit); the per-source known-flags +// are covered in their store tests, and the pixel behavior in tests/e2e. +describe("computeChatViewReveal", () => { + test("first visit holds the skeleton until BOTH history and decoration data are ready", () => { + // History still replaying, decorations unknown: skeleton, no decorations. + expect( + computeChatViewReveal({ + isHydratingTranscript: true, + chatViewDataReady: false, + hasRenderableMessages: false, + shouldShowStreamingBarrier: false, + }) + ).toEqual({ showHydrationPlaceholder: true, revealDecorations: false }); + + // Decoration data ready first (the common ordering — sources are one IPC + // round trip, replay is longer): skeleton must STILL hold so the reveal + // stays atomic. + expect( + computeChatViewReveal({ + isHydratingTranscript: true, + chatViewDataReady: true, + hasRenderableMessages: false, + shouldShowStreamingBarrier: false, + }) + ).toEqual({ showHydrationPlaceholder: true, revealDecorations: false }); + + // Both ready: one commit reveals transcript and decorations together. + expect( + computeChatViewReveal({ + isHydratingTranscript: false, + chatViewDataReady: true, + hasRenderableMessages: true, + shouldShowStreamingBarrier: false, + }) + ).toEqual({ showHydrationPlaceholder: false, revealDecorations: true }); + }); + + test("empty workspaces also wait for decoration data before revealing", () => { + // Not hydrating (no history) but sources unknown: the skeleton holds so + // the empty-placeholder + decorations appear together. + expect( + computeChatViewReveal({ + isHydratingTranscript: false, + chatViewDataReady: false, + hasRenderableMessages: false, + shouldShowStreamingBarrier: false, + }) + ).toEqual({ showHydrationPlaceholder: true, revealDecorations: false }); + }); + + test("revisits with cached rows never regress to a skeleton", () => { + // Cached rows paint immediately during incremental catch-up; latched + // known-flags make decorations renderable in that same commit. + expect( + computeChatViewReveal({ + isHydratingTranscript: true, + chatViewDataReady: true, + hasRenderableMessages: true, + shouldShowStreamingBarrier: false, + }) + ).toEqual({ showHydrationPlaceholder: false, revealDecorations: true }); + }); + + test("an active stream barrier trumps the skeleton; decorations still wait for data", () => { + const state = computeChatViewReveal({ + isHydratingTranscript: true, + chatViewDataReady: false, + hasRenderableMessages: false, + shouldShowStreamingBarrier: true, + }); + expect(state.showHydrationPlaceholder).toBe(false); + // Decorations may mount late here (reconnect-with-active-stream) — but + // they must never render while their data is merely unknown. + expect(state.revealDecorations).toBe(false); + }); +}); diff --git a/src/browser/components/ChatPane/useChatViewDataReady.ts b/src/browser/components/ChatPane/useChatViewDataReady.ts new file mode 100644 index 0000000000..1d0bf8e80b --- /dev/null +++ b/src/browser/components/ChatPane/useChatViewDataReady.ts @@ -0,0 +1,135 @@ +import { useEffect, useState } from "react"; +import { useAPI } from "@/browser/contexts/API"; +import { useBackgroundBashStateKnown } from "@/browser/stores/BackgroundBashStore"; +import { + useSessionUsageKnown, + useWorkspaceActivityHydrated, +} from "@/browser/stores/WorkspaceStore"; +import { useProvidersConfigLoaded } from "@/browser/stores/ProvidersConfigStore"; +import { + ensureAdditionalSystemContextHydrated, + useAdditionalSystemContextHydrated, +} from "@/browser/utils/additionalSystemContextStore"; + +/** + * Chat-view first-paint readiness barrier. + * + * CONTRACT — "unknown is not empty": the chat view must reveal once, fully + * formed. Any async data source that can change the chat view's INITIAL + * layout (composer-dock decorations, dock-internal banners) must: + * + * 1. distinguish "not yet loaded" from "known empty" (a store default that + * renders as "decoration absent" hides the difference and guarantees a + * pop-in when the real data lands after first paint), and + * 2. register its known-signal in this hook. + * + * ChatPane holds the transcript-hydration skeleton and suppresses the + * decoration lane until every source is known (see computeChatViewReveal), so + * the transcript and all decorations mount in one commit — layout can never + * shift because a decoration "loaded in" a few frames after the user started + * reading. Sources resolve in parallel with (and almost always faster than) + * the chat history replay, so the barrier adds no perceptible latency. + * + * Sources that DON'T need registration: + * - synchronous reads (localStorage reviews, workspace metadata); + * - chat-history-derived state (todos, queued message) — it flips in the + * same caught-up commit that reveals the transcript; + * - event-driven UI that is deterministically absent at first paint + * (context-switch warnings, edit indicators). + */ +export function useChatViewDataReady(workspaceId: string): boolean { + const { api } = useAPI(); + + // Each hook below is a `useSyncExternalStore` over a latched per-session + // "state known" flag: flags only flip false -> true, so readiness is + // monotonic for a mounted workspace and decorations are never unmounted by + // this barrier after reveal. Subscribing also starts/keeps the underlying + // backend subscription where applicable (background bashes). + const backgroundBashKnown = useBackgroundBashStateKnown(workspaceId); + const activityHydrated = useWorkspaceActivityHydrated(); + const providersConfigLoaded = useProvidersConfigLoaded(); + const sessionUsageKnown = useSessionUsageKnown(workspaceId); + const instructionsHydrated = useAdditionalSystemContextHydrated(workspaceId); + + // The scratchpad store is pull-based (historically only the Instructions + // tab hydrated it); trigger its once-per-workspace fetch here so the chat + // instructions decoration state is known before reveal. + useEffect(() => { + if (api) { + ensureAdditionalSystemContextHydrated(api, workspaceId); + } + }, [api, workspaceId]); + + const allKnown = + backgroundBashKnown && + activityHydrated && + providersConfigLoaded && + sessionUsageKnown && + instructionsHydrated; + + // Resilience deadline: every source self-heals on *error*, but a hung + // backend (no response, no rejection) has no deterministic failure signal — + // so force-reveal after a bound rather than holding the skeleton forever. + // Per AGENTS.md, startup-style initialization must never block the app: + // time out and fall back silently (decorations then mount late, which is + // exactly the pre-barrier degraded behavior). + const [forcedReadyWorkspaceId, setForcedReadyWorkspaceId] = useState(null); + useEffect(() => { + if (allKnown) { + return; + } + const timer = setTimeout(() => { + setForcedReadyWorkspaceId(workspaceId); + }, CHAT_VIEW_DATA_READY_TIMEOUT_MS); + return () => clearTimeout(timer); + }, [allKnown, workspaceId]); + + return allKnown || forcedReadyWorkspaceId === workspaceId; +} + +// Generous relative to the expected cost (each source is roughly one local +// IPC round trip resolved in parallel), tight enough that a wedged source +// degrades to "decorations pop in late" instead of "chat looks broken". +const CHAT_VIEW_DATA_READY_TIMEOUT_MS = 2_000; + +export interface ChatViewRevealInputs { + /** Transcript history replay still in flight for the active workspace. */ + isHydratingTranscript: boolean; + /** All registered decoration data sources are known (useChatViewDataReady). */ + chatViewDataReady: boolean; + /** Cached/replayed rows already renderable (revisits skip the skeleton). */ + hasRenderableMessages: boolean; + /** Active stream start/interrupt barrier is visible (trumps the skeleton). */ + shouldShowStreamingBarrier: boolean; +} + +export interface ChatViewRevealState { + /** Hold the full-transcript hydration skeleton (nothing painted yet). */ + showHydrationPlaceholder: boolean; + /** Mount layout-affecting composer decorations. */ + revealDecorations: boolean; +} + +/** + * Single reveal decision for the chat view, shared by the transcript skeleton + * and the composer decoration lane so both flip in the same commit: + * + * - First visit: skeleton holds until history is caught up AND decoration + * data is known, then everything mounts together. + * - Revisit: cached rows paint immediately (no skeleton) and the latched + * known-flags make decorations renderable in that same first commit. + * - Active stream states keep their barrier visible instead of a skeleton + * (reconnect-with-active-stream); decorations then mount when known — + * the rare case where data genuinely arrives after paint. + */ +export function computeChatViewReveal(inputs: ChatViewRevealInputs): ChatViewRevealState { + const showHydrationPlaceholder = + (inputs.isHydratingTranscript || !inputs.chatViewDataReady) && + !inputs.hasRenderableMessages && + !inputs.shouldShowStreamingBarrier; + + return { + showHydrationPlaceholder, + revealDecorations: inputs.chatViewDataReady && !showHydrationPlaceholder, + }; +} diff --git a/src/browser/features/ChatInput/CreationControls.stories.tsx b/src/browser/features/ChatInput/CreationControls.stories.tsx index ff0b950a1e..f156e61d7e 100644 --- a/src/browser/features/ChatInput/CreationControls.stories.tsx +++ b/src/browser/features/ChatInput/CreationControls.stories.tsx @@ -15,6 +15,7 @@ import { } from "@/browser/stories/mocks/coder"; import { createMockORPCClient } from "@/browser/stories/mocks/orpc"; import { useWorkspaceStoreRaw } from "@/browser/stores/WorkspaceStore"; +import { getProvidersConfigStore } from "@/browser/stores/ProvidersConfigStore"; import { RUNTIME_MODE, type ParsedRuntime, @@ -295,8 +296,12 @@ function CreationControlsStoryShell(props: { children: ReactNode }) { useEffect(() => { workspaceStore.setClient(client); + // useProvidersConfig consumers read the shared store, which gets its + // client from AppLoader in the real app — wire it manually here too. + getProvidersConfigStore().setClient(client); return () => { workspaceStore.setClient(null); + getProvidersConfigStore().setClient(null); }; }, [client, workspaceStore]); diff --git a/src/browser/features/RightSidebar/RightSidebar.stories.tsx b/src/browser/features/RightSidebar/RightSidebar.stories.tsx index 71db61a2f0..e26cad6ac1 100644 --- a/src/browser/features/RightSidebar/RightSidebar.stories.tsx +++ b/src/browser/features/RightSidebar/RightSidebar.stories.tsx @@ -29,6 +29,7 @@ import { SplashScreenProvider } from "@/browser/features/SplashScreens/SplashScr import { TerminalRouterProvider } from "@/browser/terminal/TerminalRouterContext"; import { readPersistedState, updatePersistedState } from "@/browser/hooks/usePersistedState"; import { useWorkspaceStoreRaw } from "@/browser/stores/WorkspaceStore"; +import { getProvidersConfigStore } from "@/browser/stores/ProvidersConfigStore"; import { createAssistantMessage, createUserMessage } from "@/browser/stories/mocks/messages"; import type { MockSessionUsage } from "@/browser/stories/mocks/orpc"; import { blurActiveElement } from "@/browser/stories/storyPlayHelpers"; @@ -128,8 +129,12 @@ function RightSidebarStoryShell(props: { setup: () => APIClient; children: React useEffect(() => { // App stories bypass AppLoader, so manually sync WorkspaceStore to the story client. workspaceStore.setClient(client); + // useProvidersConfig consumers read the shared store, which gets its + // client from AppLoader in the real app — wire it manually here too. + getProvidersConfigStore().setClient(client); return () => { workspaceStore.setClient(null); + getProvidersConfigStore().setClient(null); }; }, [client, workspaceStore]); diff --git a/src/browser/features/Settings/Sections/settingsStoryUtils.tsx b/src/browser/features/Settings/Sections/settingsStoryUtils.tsx index 130429c911..91b9b34aa5 100644 --- a/src/browser/features/Settings/Sections/settingsStoryUtils.tsx +++ b/src/browser/features/Settings/Sections/settingsStoryUtils.tsx @@ -13,6 +13,7 @@ import { WorkspaceProvider } from "@/browser/contexts/WorkspaceContext"; import { selectWorkspace } from "@/browser/stories/helpers/uiState"; import { createWorkspace, groupWorkspacesByProject } from "@/browser/stories/mocks/workspaces"; import { createMockORPCClient } from "@/browser/stories/mocks/orpc"; +import { getProvidersConfigStore } from "@/browser/stores/ProvidersConfigStore"; import { getExperimentKey, type ExperimentId } from "@/common/constants/experiments"; import { SELECTED_WORKSPACE_KEY, UI_THEME_KEY } from "@/common/constants/storage"; import type { ServerAuthSession } from "@/common/orpc/types"; @@ -55,6 +56,7 @@ function getStorybookRenderKey(): string | null { export function SettingsSectionStory(props: SettingsSectionStoryProps) { const lastRenderKeyRef = useRef(null); const clientRef = useRef(null); + const wiredProvidersClientRef = useRef(null); const renderKey = getStorybookRenderKey(); const shouldReset = clientRef.current === null || lastRenderKeyRef.current !== renderKey; @@ -67,6 +69,16 @@ export function SettingsSectionStory(props: SettingsSectionStoryProps) { clientRef.current ??= props.setup(); + // useProvidersConfig consumers (ProvidersSection, ModelsSection, ...) read + // the shared ProvidersConfigStore, which gets its client from AppLoader in + // the real app. These stories bypass AppLoader, so wire the store to the + // story client (once per client instance, alongside the client creation + // above, so the config fetch starts before the section mounts). + if (wiredProvidersClientRef.current !== clientRef.current) { + wiredProvidersClientRef.current = clientRef.current; + getProvidersConfigStore().setClient(clientRef.current); + } + return ( diff --git a/src/browser/hooks/useAutoScroll.test.tsx b/src/browser/hooks/useAutoScroll.test.tsx index f9bc258dc9..becd0e6d0a 100644 --- a/src/browser/hooks/useAutoScroll.test.tsx +++ b/src/browser/hooks/useAutoScroll.test.tsx @@ -35,6 +35,7 @@ function createWheelEvent( } let resizeObserverCallback: ResizeObserverCallback | null = null; +const observedTargets: Element[] = []; // Capture the hook's ResizeObserver so the safety-net pin can be driven directly. // Bottom-stick no longer uses a requestAnimationFrame settle loop: native CSS @@ -46,10 +47,11 @@ class ResizeObserverMock { } observe(target: Element): void { - void target; + observedTargets.push(target); } disconnect(): void { resizeObserverCallback = null; + observedTargets.length = 0; } } @@ -65,6 +67,7 @@ describe("useAutoScroll", () => { beforeEach(() => { cleanupDom = installDom(); resizeObserverCallback = null; + observedTargets.length = 0; window.ResizeObserver = ResizeObserverMock as unknown as typeof ResizeObserver; }); @@ -100,27 +103,36 @@ describe("useAutoScroll", () => { expect(metrics.scrollTop).toBe(metrics.maxScrollTop); }); - test("the safety-net pin is a no-op when auto-scroll is off", () => { + test("the safety net observes the scrollport and each of its children", () => { + // The in-flow composer dock sits BELOW the bottom sentinel, so native + // anchoring cannot compensate when the dock grows (decorations mounting, + // textarea growth). Observing every direct child of the scrollport is what + // re-pins that growth before paint while locked. const { result } = renderHook(() => useAutoScroll()); const element = document.createElement("div"); - const metrics = attachScrollMetrics(element, { + const transcriptContent = document.createElement("div"); + const sentinel = document.createElement("div"); + const composerDock = document.createElement("div"); + element.append(transcriptContent, sentinel, composerDock); + attachScrollMetrics(element, { scrollHeight: 1000, clientHeight: 400, - initialScrollTop: 200, }); + // Toggle autoScroll after attaching the ref so the observer effect re-runs + // (the effect bails when contentRef is null on mount). act(() => { (result.current.contentRef as MutableRefObject).current = element; result.current.disableAutoScroll(); }); - - metrics.setScrollHeight(1500); act(() => { - result.current.reanchorBottom(); + result.current.jumpToBottom(); }); - expect(metrics.scrollTop).toBe(200); - expect(result.current.autoScroll).toBe(false); + expect(observedTargets).toContain(element); + expect(observedTargets).toContain(transcriptContent); + expect(observedTargets).toContain(sentinel); + expect(observedTargets).toContain(composerDock); }); test("each layout growth re-pins to the bottom while locked", () => { @@ -157,9 +169,17 @@ describe("useAutoScroll", () => { try { let now = 1_000_000; dateNowSpy.mockImplementation(() => now); + // Toggle autoScroll after attaching the ref so the safety-net observer + // attaches (the effect bails when contentRef is null on mount). act(() => { (result.current.contentRef as MutableRefObject).current = element; + result.current.disableAutoScroll(); + }); + act(() => { + result.current.jumpToBottom(); }); + const stalePin = resizeObserverCallback; + expect(stalePin).not.toBeNull(); metrics.setScrollTop(600); act(() => { @@ -169,10 +189,10 @@ describe("useAutoScroll", () => { }); expect(result.current.autoScroll).toBe(false); - // The layout-driven safety-net pin must stay released-aware: re-anchoring - // is a no-op while unlocked, so the user keeps their position. + // The layout-driven safety-net pin must stay released-aware: a re-pin + // firing after release is a no-op, so the user keeps their position. act(() => { - result.current.reanchorBottom(); + stalePin?.([], {} as ResizeObserver); }); expect(metrics.scrollTop).toBe(600); } finally { @@ -199,9 +219,17 @@ describe("useAutoScroll", () => { let now = 1_000_000; dateNowSpy.mockImplementation(() => now); + // Toggle autoScroll after attaching the ref so the safety-net observer + // attaches (the effect bails when contentRef is null on mount). act(() => { (result.current.contentRef as MutableRefObject).current = element; + result.current.disableAutoScroll(); + }); + act(() => { + result.current.jumpToBottom(); }); + const stalePin = resizeObserverCallback; + expect(stalePin).not.toBeNull(); expect(metrics.scrollTop).toBe(metrics.maxScrollTop); // Single small wheel notch: scrollTop drops 5 px, well within the 8 px @@ -216,7 +244,7 @@ describe("useAutoScroll", () => { // A subsequent layout re-pin must not snap the user back to the bottom. act(() => { - result.current.reanchorBottom(); + stalePin?.([], {} as ResizeObserver); }); expect(metrics.scrollTop).toBe(595); } finally { @@ -242,9 +270,17 @@ describe("useAutoScroll", () => { let now = 1_000_000; dateNowSpy.mockImplementation(() => now); + // Toggle autoScroll after attaching the ref so the safety-net observer + // attaches (the effect bails when contentRef is null on mount). act(() => { (result.current.contentRef as MutableRefObject).current = element; + result.current.disableAutoScroll(); + }); + act(() => { + result.current.jumpToBottom(); }); + const stalePin = resizeObserverCallback; + expect(stalePin).not.toBeNull(); // First tick: 3 px up. Releases the lock. metrics.setScrollTop(metrics.maxScrollTop - 3); @@ -266,7 +302,7 @@ describe("useAutoScroll", () => { expect(result.current.autoScroll).toBe(false); act(() => { - result.current.reanchorBottom(); + stalePin?.([], {} as ResizeObserver); }); expect(metrics.scrollTop).toBe(594); } finally { @@ -771,15 +807,25 @@ describe("useAutoScroll", () => { initialScrollTop: 100, }); + // Toggle autoScroll after attaching the ref so the safety-net observer + // attaches (the effect bails when contentRef is null on mount). act(() => { (result.current.contentRef as MutableRefObject).current = element; + result.current.disableAutoScroll(); + }); + act(() => { result.current.jumpToBottom(); + }); + const stalePin = resizeObserverCallback; + expect(stalePin).not.toBeNull(); + + act(() => { result.current.disableAutoScroll(); }); metrics.setScrollHeight(1500); act(() => { - result.current.reanchorBottom(); + stalePin?.([], {} as ResizeObserver); }); expect(metrics.scrollTop).toBe(500); diff --git a/src/browser/hooks/useAutoScroll.ts b/src/browser/hooks/useAutoScroll.ts index 9eb6771fa0..65b8c45af1 100644 --- a/src/browser/hooks/useAutoScroll.ts +++ b/src/browser/hooks/useAutoScroll.ts @@ -81,9 +81,9 @@ function isMouseDownExemptFromScrollIntent( export function useAutoScroll() { const [autoScroll, setAutoScroll] = useState(true); const contentRef = useRef(null); - // 0-height marker rendered as the last child of the scroll content, below the - // composer-clearance padding. See the hook doc comment: while locked it is the - // sole `overflow-anchor: auto` element so native anchoring pins the bottom. + // 0-height marker rendered between the transcript content and the in-flow + // composer dock. See the hook doc comment: while locked it is the sole + // `overflow-anchor: auto` element so native anchoring pins the bottom. const sentinelRef = useRef(null); const autoScrollRef = useRef(true); const programmaticDisableRef = useRef(false); @@ -148,18 +148,6 @@ export function useAutoScroll() { seedScrollDirectionBaseline(); }, [seedScrollDirectionBaseline, setAutoScrollEnabled]); - // Re-establish the bottom after an external layout change the content - // ResizeObserver cannot see. The floating composer grows the scrollport's - // bottom *padding* (the composer clearance), which changes scrollHeight without - // resizing the scrollport's content box or the message content — so neither the - // safety-net observer nor native anchoring (the change is below the sentinel) - // re-pins. No-op unless locked, so a scrolled-up reader is never yanked down by - // composer growth. - const reanchorBottom = useCallback(() => { - if (!autoScrollRef.current) return; - stickToBottom(); - }, [stickToBottom]); - const markUserScrollIntent = useCallback(() => { programmaticDisableRef.current = false; userScrollIntentUntilRef.current = Date.now() + USER_SCROLL_INTENT_WINDOW_MS; @@ -306,11 +294,14 @@ export function useAutoScroll() { ); // Safety net behind native scroll anchoring. Anchoring (the sentinel) keeps us - // pinned as content appends, but a resize of the scrollport itself (e.g. the - // window, sidebars, or the composer-clearance padding changing) is not an - // "append above the anchor", so re-establish the bottom once per resize while - // locked. This is a single synchronous write — NOT a per-frame loop — so it - // cannot fight CSS transitions or font swaps the way the old settle loop did. + // pinned as content appends, but two layout changes are not an "append above + // the anchor": a resize of the scrollport itself (window, sidebars, onscreen + // keyboard) and growth of the in-flow composer dock, which sits BELOW the + // sentinel so anchoring cannot compensate. Observing the scrollport plus every + // direct child (transcript content, sentinel, composer dock) covers both: + // ResizeObserver delivers before paint, so this is a single synchronous + // pre-paint write per resize — NOT a per-frame loop — and cannot fight CSS + // transitions or font swaps the way the old settle loop did. useEffect(() => { if (!autoScroll) return; const scrollContainer = contentRef.current; @@ -322,9 +313,8 @@ export function useAutoScroll() { stickToBottom(); }); observer.observe(scrollContainer); - const content = scrollContainer.firstElementChild; - if (content) { - observer.observe(content); + for (const child of Array.from(scrollContainer.children)) { + observer.observe(child); } return () => observer.disconnect(); @@ -336,7 +326,6 @@ export function useAutoScroll() { autoScroll, disableAutoScroll, jumpToBottom, - reanchorBottom, handleScroll, markUserScrollIntent, handleScrollContainerWheel, diff --git a/src/browser/hooks/useProvidersConfig.ts b/src/browser/hooks/useProvidersConfig.ts index 71db535306..3b21f7651c 100644 --- a/src/browser/hooks/useProvidersConfig.ts +++ b/src/browser/hooks/useProvidersConfig.ts @@ -1,144 +1,25 @@ -import { useEffect, useState, useCallback, useRef } from "react"; -import { useAPI } from "@/browser/contexts/API"; -import type { - ProviderConfigInfo, - ProviderModelEntry, - ProvidersConfigMap, -} from "@/common/orpc/types"; +import { useSyncExternalStore } from "react"; +import { getProvidersConfigStore } from "@/browser/stores/ProvidersConfigStore"; /** * Hook to get provider config with automatic refresh on config changes. - * Subscribes to the backend's onConfigChanged event for external changes. - * Use updateOptimistically for instant UI feedback when saving. + * + * Backed by the app-wide ProvidersConfigStore (one fetch + one onConfigChanged + * subscription per app session), so after the first load the config is + * synchronously available on mount — config-derived banners can never pop in + * after first paint on workspace switches. Use updateOptimistically for + * instant UI feedback when saving; optimistic updates are visible to every + * consumer, not just the caller. */ export function useProvidersConfig() { - const { api } = useAPI(); - const [config, setConfig] = useState(null); - const [loading, setLoading] = useState(true); - - // Keep a synchronous reference to the latest config. - // - // React state updates are async, so values derived inside setState updaters - // can't be returned reliably to the caller. (We need this for the custom - // models UI, which computes an updated models array and persists it.) - const configRef = useRef(null); - // Version counter to ignore stale responses from out-of-order fetches - const fetchVersionRef = useRef(0); - - const refresh = useCallback(async () => { - if (!api) return; - const myVersion = ++fetchVersionRef.current; - try { - const cfg = await api.providers.getConfig(); - // Only update if this is the latest fetch (ignore stale responses) - if (myVersion === fetchVersionRef.current) { - configRef.current = cfg; - setConfig(cfg); - } - } catch { - // Ignore errors fetching config - } finally { - if (myVersion === fetchVersionRef.current) { - setLoading(false); - } - } - }, [api]); - - /** - * Optimistically update local state for instant UI feedback. - * Call this immediately when saving, before the API call completes. - * Bumps the fetch version to invalidate any in-flight fetches that would - * overwrite this optimistic state with stale data. - */ - const updateOptimistically = useCallback( - (provider: string, updates: Partial) => { - // Invalidate any in-flight fetches so they don't overwrite our optimistic update - fetchVersionRef.current++; - - const prev = configRef.current; - if (!prev) return; - - const next: ProvidersConfigMap = { - ...prev, - [provider]: { ...prev[provider], ...updates }, - }; - - configRef.current = next; - setConfig(next); - }, - [] - ); - - /** - * Optimistically update models for a provider. - * Returns the new models array for use in the API call. - * Bumps the fetch version to invalidate any in-flight fetches. - */ - const updateModelsOptimistically = useCallback( - ( - provider: string, - updater: (currentModels: ProviderModelEntry[]) => ProviderModelEntry[] - ): ProviderModelEntry[] => { - // Invalidate any in-flight fetches so they don't overwrite our optimistic update - fetchVersionRef.current++; - - const prev = configRef.current; - if (!prev) return []; - - const currentModels = prev[provider]?.models ?? []; - const newModels = updater(currentModels); - - const next: ProvidersConfigMap = { - ...prev, - [provider]: { ...prev[provider], models: newModels }, - }; - - configRef.current = next; - setConfig(next); - return newModels; - }, - [] - ); - - useEffect(() => { - if (!api) return; - - const abortController = new AbortController(); - const { signal } = abortController; - - // Some oRPC iterators don't eagerly close on abort alone. - // Ensure we `return()` them so backend subscriptions clean up EventEmitter listeners. - let iterator: AsyncIterator | null = null; - - // Initial fetch - void refresh(); - - // Subscribe to provider config changes via oRPC (for external changes) - (async () => { - try { - const subscribedIterator = await api.providers.onConfigChanged(undefined, { signal }); - - if (signal.aborted) { - void subscribedIterator.return?.(); - return; - } - - iterator = subscribedIterator; - - for await (const _ of subscribedIterator) { - if (signal.aborted) break; - void refresh(); - } - } catch { - // Subscription cancelled via abort signal - expected on cleanup - } - })(); - - return () => { - abortController.abort(); - void iterator?.return?.(); - }; - }, [api, refresh]); - - return { config, loading, refresh, updateOptimistically, updateModelsOptimistically }; + const store = getProvidersConfigStore(); + const config = useSyncExternalStore(store.subscribe, store.getConfig); + const loaded = useSyncExternalStore(store.subscribe, store.isLoaded); + return { + config, + loading: !loaded, + refresh: store.refresh, + updateOptimistically: store.updateOptimistically, + updateModelsOptimistically: store.updateModelsOptimistically, + }; } diff --git a/src/browser/hooks/useRouting.test.ts b/src/browser/hooks/useRouting.test.ts index a894a6258d..029531124e 100644 --- a/src/browser/hooks/useRouting.test.ts +++ b/src/browser/hooks/useRouting.test.ts @@ -6,6 +6,7 @@ import React from "react"; import { APIProvider, type APIClient } from "@/browser/contexts/API"; import { KNOWN_MODELS } from "@/common/constants/knownModels"; import type { ProvidersConfigMap } from "@/common/orpc/types"; +import { getProvidersConfigStore } from "@/browser/stores/ProvidersConfigStore"; import { useRouting } from "./useRouting"; @@ -60,6 +61,7 @@ describe("useRouting", () => { afterEach(() => { cleanup(); + getProvidersConfigStore().setClient(null); }); test("resolveRoute and availableRoutes honor gateway model accessibility", async () => { @@ -73,6 +75,12 @@ describe("useRouting", () => { }, }; + // useProvidersConfig reads the shared ProvidersConfigStore (wired by + // AppLoader in the real app); this hook test bypasses AppLoader, so wire + // it manually AFTER the stubbed config is in place so the store's fetch + // observes it. + getProvidersConfigStore().setClient(stubClient); + const { result } = renderHook(() => useRouting(), { wrapper }); await waitFor(() => { diff --git a/src/browser/stores/BackgroundBashStore.test.ts b/src/browser/stores/BackgroundBashStore.test.ts new file mode 100644 index 0000000000..4eb79065e7 --- /dev/null +++ b/src/browser/stores/BackgroundBashStore.test.ts @@ -0,0 +1,123 @@ +import { describe, expect, test } from "bun:test"; +import { BackgroundBashStore } from "./BackgroundBashStore"; +import type { APIClient } from "@/browser/contexts/API"; +import type { BackgroundProcessInfo } from "@/common/orpc/schemas/api"; + +interface BashSubscriptionState { + processes: BackgroundProcessInfo[]; + foregroundToolCallIds: string[]; +} + +const RUNNING_PROCESS: BackgroundProcessInfo = { + id: "proc-1", + pid: 4242, + script: "bun run dev", + displayName: "Dev Server", + startTime: 1_000, + status: "running", +}; + +/** + * A push-controlled stand-in for the oRPC backgroundBashes.subscribe iterator + * so tests can deliver snapshots deterministically. + */ +function createControlledBashClient() { + let push: ((state: BashSubscriptionState) => void) | null = null; + const pending: BashSubscriptionState[] = []; + + const iterator: AsyncIterableIterator = { + [Symbol.asyncIterator]() { + return this; + }, + next(): Promise> { + const queued = pending.shift(); + if (queued) { + return Promise.resolve({ value: queued, done: false }); + } + return new Promise((resolve) => { + push = (state) => { + push = null; + resolve({ value: state, done: false }); + }; + }); + }, + return(): Promise> { + return Promise.resolve({ value: undefined, done: true }); + }, + }; + + const client = { + workspace: { + backgroundBashes: { + subscribe: () => Promise.resolve(iterator), + }, + }, + } as unknown as APIClient; + + return { + client, + pushState: (state: BashSubscriptionState): void => { + if (push) { + push(state); + } else { + pending.push(state); + } + }, + }; +} + +async function waitUntil(predicate: () => boolean): Promise { + const deadline = Date.now() + 2_000; + while (!predicate()) { + if (Date.now() > deadline) throw new Error("waitUntil timed out"); + await new Promise((resolve) => setTimeout(resolve, 1)); + } +} + +describe("BackgroundBashStore state-known signal", () => { + test("unknown until the first snapshot; known and cached after, even across unsubscribe", async () => { + const { client, pushState } = createControlledBashClient(); + const store = new BackgroundBashStore(); + store.setClient(client); + + // Before any subscriber the state is simply unknown. + expect(store.isStateKnown("ws-1")).toBe(false); + + let notifications = 0; + const unsubscribe = store.subscribeStateKnown("ws-1", () => { + notifications++; + }); + + // Subscribing starts the backend subscription but the state stays unknown + // until the first snapshot actually arrives — "unknown is not empty". + expect(store.isStateKnown("ws-1")).toBe(false); + expect(store.getProcesses("ws-1")).toEqual([]); + + pushState({ processes: [RUNNING_PROCESS], foregroundToolCallIds: [] }); + await waitUntil(() => store.isStateKnown("ws-1")); + expect(notifications).toBeGreaterThan(0); + // The known-flip must observe fully applied caches. + expect(store.getProcesses("ws-1")).toEqual([RUNNING_PROCESS]); + + // Last-known state survives unsubscribe so revisiting a workspace renders + // synchronously instead of re-learning the state after first paint. + unsubscribe(); + expect(store.isStateKnown("ws-1")).toBe(true); + expect(store.getProcesses("ws-1")).toEqual([RUNNING_PROCESS]); + }); + + test("a failing subscription self-heals to known so it cannot block first paint", async () => { + const store = new BackgroundBashStore(); + store.setClient({ + workspace: { + backgroundBashes: { + subscribe: () => Promise.reject(new Error("backend down")), + }, + }, + } as unknown as APIClient); + + store.subscribeStateKnown("ws-1", () => undefined); + await waitUntil(() => store.isStateKnown("ws-1")); + expect(store.getProcesses("ws-1")).toEqual([]); + }); +}); diff --git a/src/browser/stores/BackgroundBashStore.ts b/src/browser/stores/BackgroundBashStore.ts index 29f9a67a3d..b48c469fec 100644 --- a/src/browser/stores/BackgroundBashStore.ts +++ b/src/browser/stores/BackgroundBashStore.ts @@ -40,11 +40,19 @@ export class BackgroundBashStore { private processesStore = new MapStore(); private foregroundIdsStore = new MapStore>(); private terminatingIdsStore = new MapStore>(); + private stateKnownStore = new MapStore(); private processesCache = new Map(); private autoBackgroundFetches = new Map>(); private foregroundIdsCache = new Map>(); private terminatingIdsCache = new Map>(); + // Workspaces whose background-bash state is KNOWN (the live subscription + // delivered at least one snapshot, or it failed and we self-healed). + // The chat view's first-paint barrier (useChatViewDataReady) waits on this + // so the banner can never pop in after the transcript reveals: an empty + // process list is only renderable as "no banner" once it is known-empty + // rather than not-yet-loaded. Kept across unsubscribes (last-known state). + private stateKnownWorkspaces = new Set(); private subscriptions = new Map< string, @@ -110,6 +118,34 @@ export class BackgroundBashStore { }; }; + /** + * Subscribe to the "state known" signal. Like the data subscriptions, this + * ref-counts the live backend subscription — so the chat view's readiness + * barrier both observes AND drives the initial snapshot fetch, keeping the + * per-workspace subscription warm for the whole chat pane lifetime even + * while the banner itself renders nothing. + */ + subscribeStateKnown = (workspaceId: string, listener: () => void): (() => void) => { + this.trackSubscription(workspaceId); + const unsubscribe = this.stateKnownStore.subscribeKey(workspaceId, listener); + return () => { + unsubscribe(); + this.untrackSubscription(workspaceId); + }; + }; + + isStateKnown(workspaceId: string): boolean { + return this.stateKnownStore.get(workspaceId, () => this.stateKnownWorkspaces.has(workspaceId)); + } + + private markStateKnown(workspaceId: string): void { + if (this.stateKnownWorkspaces.has(workspaceId)) { + return; + } + this.stateKnownWorkspaces.add(workspaceId); + this.stateKnownStore.bump(workspaceId); + } + getProcesses(workspaceId: string): BackgroundProcessInfo[] { return this.processesStore.get( workspaceId, @@ -271,12 +307,12 @@ export class BackgroundBashStore { this.clearRetry(workspaceId); - this.processesCache.delete(workspaceId); - this.foregroundIdsCache.delete(workspaceId); - this.terminatingIdsCache.delete(workspaceId); - this.processesStore.delete(workspaceId); - this.foregroundIdsStore.delete(workspaceId); - this.terminatingIdsStore.delete(workspaceId); + // Intentionally KEEP the per-workspace caches and the state-known flag. + // Revisiting a workspace then renders the last-known state synchronously + // at first paint (the fresh subscription reconciles within a tick) instead + // of re-learning "are there background bashes?" after paint — which made + // the banner pop in and shift the transcript on every workspace switch. + // The retained data is a handful of small lists per visited workspace. } private clearRetry(workspaceId: string): void { @@ -372,10 +408,18 @@ export class BackgroundBashStore { this.terminatingIdsStore.bump(workspaceId); } } + + // Mark known AFTER applying the snapshot so observers that wake on + // the known-flip read fully-populated caches. + this.markStateKnown(workspaceId); } } catch (err) { if (!signal.aborted && !isAbortError(err)) { console.error("Failed to subscribe to background bash state:", err); + // Self-heal: a broken subscription must not hold the chat view's + // first-paint barrier — treat the state as known (empty) and let + // the retry deliver real data later. + this.markStateKnown(workspaceId); } } finally { void subscription.iterator?.return?.(); @@ -454,3 +498,17 @@ export function useBackgroundBashTerminatingIds(workspaceId: string | undefined) () => (workspaceId ? store.getTerminatingIds(workspaceId) : EMPTY_SET) ); } + +/** + * True once this workspace's background-bash state is known (first snapshot + * received this app session, or self-healed after a subscription failure). + * Subscribing also keeps the live backend subscription alive — see + * subscribeStateKnown. + */ +export function useBackgroundBashStateKnown(workspaceId: string): boolean { + const store = getStoreInstance(); + return useSyncExternalStore( + (listener) => store.subscribeStateKnown(workspaceId, listener), + () => store.isStateKnown(workspaceId) + ); +} diff --git a/src/browser/stores/ProvidersConfigStore.test.ts b/src/browser/stores/ProvidersConfigStore.test.ts new file mode 100644 index 0000000000..cb69b20a0e --- /dev/null +++ b/src/browser/stores/ProvidersConfigStore.test.ts @@ -0,0 +1,92 @@ +import { describe, expect, mock, test } from "bun:test"; +import { ProvidersConfigStore } from "./ProvidersConfigStore"; +import type { APIClient } from "@/browser/contexts/API"; +import type { ProvidersConfigMap } from "@/common/orpc/types"; + +const SAMPLE_CONFIG: ProvidersConfigMap = { + anthropic: { + apiKeySet: true, + isEnabled: true, + isConfigured: true, + models: [], + }, +}; + +function createClient(getConfig: () => Promise): Pick { + return { + providers: { + getConfig, + // Keep the change subscription open without ever yielding so tests + // exercise the fetch/optimistic paths deterministically. + onConfigChanged: async function* () { + yield* []; + await new Promise(() => undefined); + }, + }, + } as unknown as Pick; +} + +async function waitUntil(predicate: () => boolean): Promise { + const deadline = Date.now() + 2_000; + while (!predicate()) { + if (Date.now() > deadline) throw new Error("waitUntil timed out"); + await new Promise((resolve) => setTimeout(resolve, 1)); + } +} + +describe("ProvidersConfigStore", () => { + test("loads once per client and serves every subscriber synchronously after", async () => { + const getConfig = mock(() => Promise.resolve(SAMPLE_CONFIG)); + const store = new ProvidersConfigStore(); + + expect(store.isLoaded()).toBe(false); + store.setClient(createClient(getConfig) as APIClient); + + const notified = mock(() => undefined); + store.subscribe(notified); + store.subscribe(() => undefined); + + await waitUntil(() => store.isLoaded()); + expect(store.getConfig()).toEqual(SAMPLE_CONFIG); + expect(notified).toHaveBeenCalled(); + // Subscribing more consumers must not trigger more fetches. + expect(getConfig).toHaveBeenCalledTimes(1); + }); + + test("a failed fetch still marks the store loaded (self-heal, no stuck loading)", async () => { + const store = new ProvidersConfigStore(); + store.setClient(createClient(() => Promise.reject(new Error("backend down"))) as APIClient); + + await waitUntil(() => store.isLoaded()); + expect(store.getConfig()).toBeNull(); + }); + + test("optimistic updates win over stale in-flight fetch responses", async () => { + let resolveSlowFetch: ((config: ProvidersConfigMap) => void) | null = null; + let calls = 0; + const getConfig = () => { + calls++; + if (calls === 1) { + return Promise.resolve(SAMPLE_CONFIG); + } + return new Promise((resolve) => { + resolveSlowFetch = resolve; + }); + }; + + const store = new ProvidersConfigStore(); + store.setClient(createClient(getConfig) as APIClient); + await waitUntil(() => store.isLoaded()); + + // Start a slow refresh, then land an optimistic update while it is in flight. + void store.refresh(); + await waitUntil(() => resolveSlowFetch !== null); + store.updateOptimistically("anthropic", { apiKeySet: false }); + expect(store.getConfig()?.anthropic?.apiKeySet).toBe(false); + + // The stale response must NOT clobber the optimistic state. + resolveSlowFetch!(SAMPLE_CONFIG); + await new Promise((resolve) => setTimeout(resolve, 5)); + expect(store.getConfig()?.anthropic?.apiKeySet).toBe(false); + }); +}); diff --git a/src/browser/stores/ProvidersConfigStore.ts b/src/browser/stores/ProvidersConfigStore.ts new file mode 100644 index 0000000000..bca10c9af3 --- /dev/null +++ b/src/browser/stores/ProvidersConfigStore.ts @@ -0,0 +1,196 @@ +import { useSyncExternalStore } from "react"; +import type { APIClient } from "@/browser/contexts/API"; +import type { + ProviderConfigInfo, + ProviderModelEntry, + ProvidersConfigMap, +} from "@/common/orpc/types"; + +/** + * App-wide shared cache of the providers config. + * + * Previously every `useProvidersConfig()` consumer (a dozen components, + * including ChatPane and ChatInput) issued its own `providers.getConfig()` + * fetch plus its own `onConfigChanged` subscription on mount. Besides the + * IPC fan-out, the per-mount fetch meant `config` was `null` for the first + * frames of EVERY mount — so config-derived UI (CompactionWarning, + * CodexOauthWarningBanner) popped in after first paint on every workspace + * switch, visibly shifting the composer dock. + * + * One store = one fetch + one subscription per app session, and after the + * first load the config is synchronously available to every consumer. The + * `isLoaded` signal participates in the chat view's first-paint readiness + * barrier (see useChatViewDataReady). + */ +export class ProvidersConfigStore { + private client: APIClient | null = null; + private config: ProvidersConfigMap | null = null; + private loaded = false; + private listeners = new Set<() => void>(); + // Version counter to ignore stale responses from out-of-order fetches + // (and to invalidate in-flight fetches when an optimistic update lands). + private fetchVersion = 0; + private subscriptionController: AbortController | null = null; + // Live onConfigChanged iterator. Kept on the instance (not just in the + // subscription closure) so setClient can force-close it: some oRPC + // iterators don't eagerly close on abort alone, and leaving the old one + // open across client swaps/reconnects leaks backend EventEmitter listeners + // and keeps stale refresh loops alive. + private subscriptionIterator: AsyncIterator | null = null; + + setClient(client: APIClient | null): void { + this.client = client; + + this.subscriptionController?.abort(); + this.subscriptionController = null; + void this.subscriptionIterator?.return?.(); + this.subscriptionIterator = null; + // Invalidate in-flight fetches from the previous client. + this.fetchVersion++; + + if (!client) { + return; + } + + void this.refresh(); + this.runConfigChangedSubscription(client); + } + + subscribe = (listener: () => void): (() => void) => { + this.listeners.add(listener); + return () => { + this.listeners.delete(listener); + }; + }; + + getConfig = (): ProvidersConfigMap | null => this.config; + + isLoaded = (): boolean => this.loaded; + + refresh = async (): Promise => { + const client = this.client; + if (!client) return; + const myVersion = ++this.fetchVersion; + try { + const cfg = await client.providers.getConfig(); + // Only update if this is the latest fetch (ignore stale responses). + if (myVersion === this.fetchVersion) { + this.config = cfg; + this.notify(); + } + } catch { + // Ignore errors fetching config. + } finally { + // Mark loaded even on failure so consumers (and the chat first-paint + // barrier) never block on a broken config fetch — self-healing over + // stuck loading states. + if (myVersion === this.fetchVersion && !this.loaded) { + this.loaded = true; + this.notify(); + } + } + }; + + /** + * Optimistically update local state for instant UI feedback. + * Call this immediately when saving, before the API call completes. + * Bumps the fetch version to invalidate any in-flight fetches that would + * overwrite this optimistic state with stale data. + */ + updateOptimistically = (provider: string, updates: Partial): void => { + this.fetchVersion++; + + const prev = this.config; + if (!prev) return; + + this.config = { + ...prev, + [provider]: { ...prev[provider], ...updates }, + }; + this.notify(); + }; + + /** + * Optimistically update models for a provider. + * Returns the new models array for use in the API call. + * Bumps the fetch version to invalidate any in-flight fetches. + */ + updateModelsOptimistically = ( + provider: string, + updater: (currentModels: ProviderModelEntry[]) => ProviderModelEntry[] + ): ProviderModelEntry[] => { + this.fetchVersion++; + + const prev = this.config; + if (!prev) return []; + + const currentModels = prev[provider]?.models ?? []; + const newModels = updater(currentModels); + + this.config = { + ...prev, + [provider]: { ...prev[provider], models: newModels }, + }; + this.notify(); + return newModels; + }; + + private notify(): void { + for (const listener of this.listeners) { + listener(); + } + } + + private runConfigChangedSubscription(client: APIClient): void { + const controller = new AbortController(); + const { signal } = controller; + this.subscriptionController = controller; + + let iterator: AsyncIterator | null = null; + + void (async () => { + try { + const subscribedIterator = await client.providers.onConfigChanged(undefined, { signal }); + + // If the client was swapped while subscribe() was in flight, + // force-close immediately so the backend drops its listener. + if (signal.aborted || this.subscriptionController !== controller) { + void subscribedIterator.return?.(); + return; + } + + iterator = subscribedIterator; + // Publish so setClient can return() it (see subscriptionIterator). + this.subscriptionIterator = subscribedIterator; + + for await (const _ of subscribedIterator) { + if (signal.aborted) break; + void this.refresh(); + } + } catch { + // Subscription cancelled via abort signal - expected on cleanup. + } finally { + void iterator?.return?.(); + if (this.subscriptionIterator === iterator) { + this.subscriptionIterator = null; + } + } + })(); + } +} + +let storeInstance: ProvidersConfigStore | null = null; + +export function getProvidersConfigStore(): ProvidersConfigStore { + storeInstance ??= new ProvidersConfigStore(); + return storeInstance; +} + +/** + * True once the providers config has been fetched (or the fetch failed) for + * the current app session. Synchronously true afterwards for every consumer. + */ +export function useProvidersConfigLoaded(): boolean { + const store = getProvidersConfigStore(); + return useSyncExternalStore(store.subscribe, store.isLoaded); +} diff --git a/src/browser/stores/WorkspaceStore.ts b/src/browser/stores/WorkspaceStore.ts index d5fe6623dd..1fbbc729b3 100644 --- a/src/browser/stores/WorkspaceStore.ts +++ b/src/browser/stores/WorkspaceStore.ts @@ -653,6 +653,27 @@ export class WorkspaceStore { // abort/error transitions (streaming=false without recency advance). private activityStreamingStartRecency = new Map(); private activityAbortController: AbortController | null = null; + // True once the initial activity.list() snapshot has been applied (or the + // subscription failed and we self-healed). Until then, "no other workspace + // is streaming" is merely unknown — the chat view's first-paint barrier + // (useChatViewDataReady) waits on this so cross-workspace decorations like + // the concurrent-local warning can't pop in after the transcript reveals. + private activityHydrated = false; + private activityHydratedListeners = new Set<() => void>(); + // Workspaces whose persisted session usage fetch has settled (success or + // failure). Distinguishes "usage unknown" from "no usage" for the same + // first-paint barrier (CompactionWarning derives from usage). + // + // Deliberately LATCHED per session: revisiting a workspace re-fetches usage + // (setActiveWorkspaceId), but the barrier keeps revealing with last-known + // usage rather than waiting for the refresh. Resetting this per refresh + // would flip useChatViewDataReady false on every workspace switch and + // unmount/remount the whole composer decoration lane (a guaranteed flash) + // to guard against a rare one (usage changed while inactive AND crossed the + // compaction threshold — a genuine live state change, allowed to move UI + // like any other live event). The barrier's contract is "never paint + // unknown as absent", not "freeze data that legitimately updates". + private sessionUsageKnown = new Set(); private activeGoalCount = 0; private activeGoalCountStore = new MapStore(); @@ -1057,10 +1078,10 @@ export class WorkspaceStore { client.workspace .getSessionUsage({ workspaceId }) .then((data) => { - if (!data) { - return; - } - // Stale-response guard: a newer refresh was issued while this one was in-flight. + // Stale-response guard: a newer refresh was issued while this one was + // in-flight. The newer request owns the "known" flip too — flipping it + // here would let the chat view reveal before the latest usage landed, + // re-opening the post-reveal pop-in this barrier exists to prevent. if ((this.sessionUsageRequestVersion.get(workspaceId) ?? 0) !== requestVersion) { return; } @@ -1068,6 +1089,14 @@ export class WorkspaceStore { if (!this.isWorkspaceRegistered(workspaceId)) { return; } + // The latest settled fetch makes the usage "known" (including a null + // response = known-empty) — the chat view's first-paint barrier + // distinguishes that from not-yet-loaded so usage-derived banners + // can't pop in after reveal. + this.markSessionUsageKnown(workspaceId); + if (!data) { + return; + } if ( this.providersConfig && @@ -1082,6 +1111,12 @@ export class WorkspaceStore { }) .catch((error) => { console.warn(`Failed to fetch session usage for ${workspaceId}:`, error); + // Self-heal: a failed LATEST fetch must not hold the first-paint + // barrier. Stale failures defer to the newer in-flight request, same + // as the success path above. + if ((this.sessionUsageRequestVersion.get(workspaceId) ?? 0) === requestVersion) { + this.markSessionUsageKnown(workspaceId); + } }); } @@ -3090,6 +3125,38 @@ export class WorkspaceStore { } } + private markActivityHydrated(): void { + if (this.activityHydrated) { + return; + } + this.activityHydrated = true; + for (const listener of this.activityHydratedListeners) { + listener(); + } + } + + isActivityHydrated = (): boolean => this.activityHydrated; + + subscribeActivityHydrated = (listener: () => void): (() => void) => { + this.activityHydratedListeners.add(listener); + return () => { + this.activityHydratedListeners.delete(listener); + }; + }; + + isSessionUsageKnown(workspaceId: string): boolean { + return this.sessionUsageKnown.has(workspaceId); + } + + private markSessionUsageKnown(workspaceId: string): void { + if (this.sessionUsageKnown.has(workspaceId)) { + return; + } + this.sessionUsageKnown.add(workspaceId); + // Reuse the usage channel so useSessionUsageKnown subscribers wake up. + this.usageStore.bump(workspaceId); + } + private async runActivitySubscription(signal: AbortSignal): Promise { let attempt = 0; @@ -3126,6 +3193,7 @@ export class WorkspaceStore { return; } this.applyWorkspaceActivityList(snapshots); + this.markActivityHydrated(); }); // Start watchdog after bootstrap so slow list() doesn't trigger @@ -3173,6 +3241,10 @@ export class WorkspaceStore { } } else if (!abortError) { console.warn("[WorkspaceStore] Error in activity subscription:", error); + // Self-heal: a failing activity subscription must not hold the chat + // view's first-paint barrier. Treat the (empty) activity map as + // known; the retry loop will deliver real data when it recovers. + this.markActivityHydrated(); } } finally { releaseAttemptListeners(); @@ -4627,6 +4699,29 @@ export function useWorkspaceUsage(workspaceId: string): WorkspaceUsageState { ); } +/** + * True once the workspace's persisted session-usage fetch has settled this + * app session (success or failure). Lets the chat view's first-paint barrier + * distinguish "usage unknown" from "no usage" so usage-derived banners + * (CompactionWarning) can't pop in after the transcript reveals. + */ +export function useSessionUsageKnown(workspaceId: string): boolean { + const store = getStoreInstance(); + return useSyncExternalStore( + (listener) => store.subscribeUsage(workspaceId, listener), + () => store.isSessionUsageKnown(workspaceId) + ); +} + +/** + * True once the initial cross-workspace activity snapshot has been applied + * (or its subscription self-healed after failure). See activityHydrated. + */ +export function useWorkspaceActivityHydrated(): boolean { + const store = getStoreInstance(); + return useSyncExternalStore(store.subscribeActivityHydrated, store.isActivityHydrated); +} + /** * Hook for the live token-count / TPS pill during streaming. * diff --git a/src/browser/utils/additionalSystemContextStore.ts b/src/browser/utils/additionalSystemContextStore.ts index 00c7602841..624f9af906 100644 --- a/src/browser/utils/additionalSystemContextStore.ts +++ b/src/browser/utils/additionalSystemContextStore.ts @@ -71,6 +71,47 @@ export function subscribeAdditionalSystemContext( }; } +const hydrationInFlight = new Set(); + +/** + * Fetch the persisted scratchpad once per workspace per app session. + * + * Historically the store was only hydrated by the Instructions-tab editor, so + * the chat-input decoration for saved instructions stayed hidden until that + * tab was opened — and then popped in. The chat view's first-paint barrier + * (useChatViewDataReady) calls this at workspace open so the decoration's + * state is known before the transcript reveals. Idempotent and re-entrant + * safe; never rejects (a failed fetch marks the store hydrated-empty so first + * paint is never blocked — self-healing over a stuck skeleton). + */ +export function ensureAdditionalSystemContextHydrated(api: APIClient, workspaceId: string): void { + if (isAdditionalSystemContextHydrated(workspaceId) || hydrationInFlight.has(workspaceId)) { + return; + } + hydrationInFlight.add(workspaceId); + + api.workspace + .getAdditionalSystemContext({ workspaceId }) + .then((result) => { + // A concurrent hydration (e.g. the Instructions editor) or a local edit + // may have landed first — never clobber it with this fetch. + if (!isAdditionalSystemContextHydrated(workspaceId)) { + updateAdditionalSystemContextSnapshot(workspaceId, { + content: result.content, + enabled: result.enabled, + }); + } + }) + .catch(() => { + if (!isAdditionalSystemContextHydrated(workspaceId)) { + updateAdditionalSystemContextSnapshot(workspaceId, DEFAULT_SNAPSHOT); + } + }) + .finally(() => { + hydrationInFlight.delete(workspaceId); + }); +} + interface SaveCallbacks { onError?: (error: unknown) => void; onIdle?: () => void; diff --git a/tests/e2e/scenarios/composerLayoutStability.spec.ts b/tests/e2e/scenarios/composerLayoutStability.spec.ts index 3d9ea42a9d..b8761eb410 100644 --- a/tests/e2e/scenarios/composerLayoutStability.spec.ts +++ b/tests/e2e/scenarios/composerLayoutStability.spec.ts @@ -2,13 +2,15 @@ import { electronTest as test, electronExpect as expect } from "../electronTest" import { MOCK_LIST_PROGRAMMING_LANGUAGES } from "../mockAiPrompts"; // Real-browser gate for the chat-send "layout flash" fix. happy-dom cannot evaluate -// native CSS scroll anchoring, calc(var(--composer-h)) clearance, or per-frame -// layout geometry, so the architecture's core invariants are proven here: +// native CSS scroll anchoring, sticky positioning, or per-frame layout geometry, so +// the architecture's core invariants are proven here: // -// Pillar D — the composer floats out of flow, so growing/collapsing it (the -// send-time root cause) NEVER changes the transcript scrollport's clientHeight. +// Pillar D — the composer dock is in-flow scroll content (sticky to the +// scrollport bottom), so growing/collapsing it (the send-time root cause) NEVER +// changes the transcript scrollport's clientHeight, and the clearance for the +// last message is reserved by flow layout in the same pass as the height change. // Pillar A — the bottom sentinel + native anchoring keep the transcript pinned to -// the bottom with the last message clear of the floating composer. +// the bottom with the last message clear of the composer dock. // // clientHeight is a stable per-frame layout property (not a transient paint), so // sampling it across the send is deterministic rather than flaky. @@ -44,7 +46,8 @@ test("composer height changes never resize the transcript viewport, and the bott expect(baseline).toBeGreaterThan(0); // Pillar D (grow): a tall multi-line draft must enlarge the composer WITHOUT - // shrinking the scrollport, and the scrollport must reserve matching clearance. + // shrinking the scrollport, and flow layout must keep the last message clear of + // the taller dock (re-pinned by anchoring/ResizeObserver before paint). await input.fill(TALL_DRAFT); const grown = await page.evaluate( ({ win, dock }) => { @@ -53,7 +56,6 @@ test("composer height changes never resize the transcript viewport, and the bott return { clientHeight: sp.clientHeight, composerHeight: Math.round(composer.getBoundingClientRect().height), - paddingBottom: Number.parseFloat(getComputedStyle(sp).paddingBottom), }; }, { win: MESSAGE_WINDOW, dock: COMPOSER_DOCK } @@ -61,8 +63,26 @@ test("composer height changes never resize the transcript viewport, and the bott expect(grown.composerHeight).toBeGreaterThan(0); // The headline invariant: a taller composer does not steal scrollport height. expect(grown.clientHeight).toBe(baseline); - // The clearance padding tracks the live composer height (calc(var(--composer-h))). - expect(grown.paddingBottom).toBeGreaterThanOrEqual(grown.composerHeight); + // The in-flow dock reserves its own clearance: the transcript stays pinned with + // the last message above the grown dock. + await expect + .poll(() => + page.evaluate( + ({ win, dock }) => { + const sp = document.querySelector(win)!; + const composer = document.querySelector(dock)!; + const rows = sp.querySelectorAll("[data-message-id]"); + const last = rows[rows.length - 1]; + if (!last) return false; + const pinnedToBottom = sp.scrollHeight - sp.clientHeight - sp.scrollTop <= 4; + const lastClearsComposer = + last.getBoundingClientRect().bottom <= composer.getBoundingClientRect().top + 2; + return pinnedToBottom && lastClearsComposer; + }, + { win: MESSAGE_WINDOW, dock: COMPOSER_DOCK } + ) + ) + .toBe(true); // Pillar D (collapse): clearing the draft collapses the composer; still no resize. await input.fill(""); @@ -110,9 +130,9 @@ test("composer height changes never resize the transcript viewport, and the bott expect(sample).toBe(baseline); } - // Pillar A: after settling, the transcript is pinned to the bottom, the sentinel is - // the last child, and the last message clears the floating composer — all while the - // viewport height is still the baseline. + // Pillar A: after settling, the transcript is pinned to the bottom, the dock is + // the last child with the sentinel directly above it, and the last message clears + // the composer dock — all while the viewport height is still the baseline. await expect .poll( () => @@ -125,13 +145,15 @@ test("composer height changes never resize the transcript viewport, and the bott const last = rows[rows.length - 1]; if (!last) return false; const pinnedToBottom = sp.scrollHeight - sp.clientHeight - sp.scrollTop <= 4; - const sentinelIsLast = sp.lastElementChild === sentinelEl; + const dockIsLast = sp.lastElementChild === composer; + const sentinelAboveDock = sentinelEl.nextElementSibling === composer; const lastClearsComposer = last.getBoundingClientRect().bottom <= composer.getBoundingClientRect().top + 2; return ( sp.clientHeight === baselineHeight && pinnedToBottom && - sentinelIsLast && + dockIsLast && + sentinelAboveDock && lastClearsComposer ); }, diff --git a/tests/ui/chat/floatingComposerAnchoring.test.ts b/tests/ui/chat/composerDockAnchoring.test.ts similarity index 64% rename from tests/ui/chat/floatingComposerAnchoring.test.ts rename to tests/ui/chat/composerDockAnchoring.test.ts index fbbf0eb667..6c19fb2f10 100644 --- a/tests/ui/chat/floatingComposerAnchoring.test.ts +++ b/tests/ui/chat/composerDockAnchoring.test.ts @@ -21,22 +21,27 @@ function getMessageWindow(container: HTMLElement): HTMLDivElement { return element as HTMLDivElement; } -// These tests encode the structural contract behind the "send flash" fix: -// 1. The composer floats (absolute) in its own subtree, so its height changes never -// resize the transcript scrollport (the root cause of viewport-resize-from-below). -// 2. A 0-height bottom sentinel is the LAST child of the scrollport and the sole -// `overflow-anchor: auto` element while locked, so native CSS scroll anchoring -// pins the bottom on append without a JS settle loop. +// These tests encode the structural contract behind the "send flash" + "hydration +// tear" fixes: +// 1. The composer dock is IN-FLOW scroll content stuck (position: sticky) to the +// scrollport bottom, so transcript clearance is reserved by flow layout in the +// same layout pass as any dock height change (no measured --composer-h channel +// that can lag a frame and tear), while the scrollport's clientHeight never +// changes when the composer resizes. +// 2. A 0-height bottom sentinel sits between the transcript content and the dock +// and is the sole `overflow-anchor: auto` element while locked, so native CSS +// scroll anchoring pins the bottom on append without a JS settle loop. The +// dock itself must never be an anchor candidate. // 3. Releasing the lock (scrolling up) restores row anchoring so the reading // position is preserved. // happy-dom cannot exercise real native anchoring, so these assert the DOM/style // contract the browser relies on; the pixel behavior is covered by the e2e suite. -describe("Floating composer + bottom anchoring", () => { +describe("Sticky composer dock + bottom anchoring", () => { beforeAll(async () => { await preloadTestModules(); }); - test("floats the composer, reserves clearance, and keeps the sentinel as the sole bottom anchor", async () => { + test("keeps the dock in-flow below the sentinel and the sentinel as the sole bottom anchor", async () => { const app = await createAppHarness({ branchPrefix: "floating-composer-anchor" }); try { @@ -45,20 +50,23 @@ describe("Floating composer + bottom anchoring", () => { const messageWindow = getMessageWindow(app.view.container); - // (1) The composer lives in a separate, floating subtree — not a flex sibling - // nested in (or wrapping) the scrollport. (The scrollport's clearance padding - // uses calc(var(--composer-h)), which happy-dom drops; the pixel clearance is - // asserted in the e2e suite where a real layout engine evaluates it.) + // (1) The composer dock is in-flow scroll content: the last child of the + // scrollport, stuck to its bottom, and opted out of scroll anchoring so the + // browser can never anchor to the sticky dock. const dock = app.view.container.querySelector('[data-testid="chat-composer-dock"]'); if (!dock) throw new Error("Composer dock not found"); - expect(messageWindow.contains(dock)).toBe(false); - expect(dock.contains(messageWindow)).toBe(false); - expect(dock.classList.contains("absolute")).toBe(true); + expect(messageWindow.contains(dock)).toBe(true); + expect(dock.parentElement).toBe(messageWindow); + expect(messageWindow.lastElementChild).toBe(dock); + expect(dock.classList.contains("sticky")).toBe(true); + expect((dock as HTMLElement).style.overflowAnchor).toBe("none"); - // (2) The sentinel is the last child of the scrollport and is the anchor. + // (2) The sentinel sits immediately above the dock: appends grow content + // ABOVE it (native anchoring pins the bottom) while dock growth happens + // BELOW it (covered by the scrollport-children ResizeObserver). const sentinel = messageWindow.querySelector('[data-testid="transcript-bottom-sentinel"]'); if (!sentinel) throw new Error("Bottom sentinel not found"); - expect(messageWindow.lastElementChild).toBe(sentinel); + expect(sentinel.nextElementSibling).toBe(dock); expect((sentinel as HTMLElement).style.overflowAnchor).toBe("auto"); // (2) While locked (at the bottom after a send) the transcript content opts OUT diff --git a/tests/ui/config/customModels.test.ts b/tests/ui/config/customModels.test.ts index 16babdbe11..d6c87c1cf3 100644 --- a/tests/ui/config/customModels.test.ts +++ b/tests/ui/config/customModels.test.ts @@ -21,6 +21,7 @@ import { cleanup, render, waitFor } from "@testing-library/react"; import { APIProvider, useAPI } from "@/browser/contexts/API"; import { useProvidersConfig } from "@/browser/hooks/useProvidersConfig"; +import { getProvidersConfigStore } from "@/browser/stores/ProvidersConfigStore"; import type { ProviderModelEntry } from "@/common/orpc/types"; import { getProviderModelEntryId } from "@/common/utils/providers/modelEntries"; @@ -85,6 +86,11 @@ describeIntegration("Custom Models", () => { const cleanupDom = installDom(); + // useProvidersConfig reads the shared ProvidersConfigStore, which gets its + // client from AppLoader in the real app. This harness bypasses AppLoader + // (see module doc comment), so wire the store manually. + getProvidersConfigStore().setClient(env.orpc); + // Ensure starting from a clean slate. const reset = await env.orpc.providers.setModels({ provider: "anthropic", models: [] }); if (!reset.success) { @@ -130,6 +136,7 @@ describeIntegration("Custom Models", () => { view.unmount(); cleanup(); cleanupDom(); + getProvidersConfigStore().setClient(null); // Best-effort cleanup so other tests don't inherit custom models. await env.orpc.providers.setModels({ provider: "anthropic", models: [] });