diff --git a/web/oss/src/components/AgentChatSlice/AgentConversation.tsx b/web/oss/src/components/AgentChatSlice/AgentConversation.tsx index 8609c3d6c0..561c0da698 100644 --- a/web/oss/src/components/AgentChatSlice/AgentConversation.tsx +++ b/web/oss/src/components/AgentChatSlice/AgentConversation.tsx @@ -9,7 +9,7 @@ import { import {simulatedAgentRunAtomFamily} from "@agenta/shared/state" import {type RichChatInputHandle} from "@agenta/ui/rich-chat-input" import {UploadSimple} from "@phosphor-icons/react" -import {type UIMessage} from "ai" +import {type FileUIPart, type UIMessage} from "ai" import {Modal} from "antd" import {useAtomValue, useSetAtom, useStore} from "jotai" @@ -411,7 +411,23 @@ const AgentConversation = ({ ] if (!trimmed && fileObjs.length === 0) return if (!attachmentsSettled) return - const fileParts = fileObjs.length ? await filesToParts(fileObjs) : undefined + let fileParts: FileUIPart[] | undefined + if (fileObjs.length) { + const {parts, unreadable} = await filesToParts(fileObjs) + // Hold the send rather than quietly dropping bytes the user staged, and say which file + // failed through the same inline channel the other attachment refusals use. + if (unreadable.length) { + attachments.setRejections( + unreadable.map((f) => ({ + name: f.name, + reason: "couldn't be read — remove it and attach it again", + })), + ) + attachments.setAttachmentsOpen(true) + return + } + fileParts = parts + } // Glide to the bottom; the min-h-full active turn makes that show the new question at the top // with the answer streaming below. Park during the glide, follow again on settle. Clear any // prior "stopped" marker — it's resolved by asking again. diff --git a/web/oss/src/components/AgentChatSlice/assets/files.ts b/web/oss/src/components/AgentChatSlice/assets/files.ts index 8e710d0a12..243664027b 100644 --- a/web/oss/src/components/AgentChatSlice/assets/files.ts +++ b/web/oss/src/components/AgentChatSlice/assets/files.ts @@ -32,9 +32,29 @@ const fileToPart = (file: File): Promise => reader.readAsDataURL(file) }) -/** Convert picked `File`s into `file` parts for `sendMessage({text, files})`. */ -export const filesToParts = (files: File[]): Promise => - Promise.all(files.map(fileToPart)) +export interface FileReadResult { + parts: FileUIPart[] + /** Files the browser refused to read (revoked blob, moved/locked on disk), in input order. */ + unreadable: File[] +} + +/** + * Convert picked `File`s into `file` parts for `sendMessage({text, files})`. + * + * Never rejects. Every send path drops this promise (composer submit, voice take, empty-state + * Start, the first-run seed), so a `Promise.all` that threw on one unreadable file surfaced as an + * unhandled rejection and a send that silently did nothing. Failures come back as data instead. + */ +export const filesToParts = async (files: File[]): Promise => { + const settled = await Promise.allSettled(files.map(fileToPart)) + const parts: FileUIPart[] = [] + const unreadable: File[] = [] + settled.forEach((result, i) => { + if (result.status === "fulfilled") parts.push(result.value) + else unreadable.push(files[i]) + }) + return {parts, unreadable} +} /** The `file` parts of a message, in order. */ export const fileParts = (message: UIMessage): FileUIPart[] => diff --git a/web/oss/src/components/AgentChatSlice/components/AgentTranscript.tsx b/web/oss/src/components/AgentChatSlice/components/AgentTranscript.tsx index 443a1e7214..c464457e68 100644 --- a/web/oss/src/components/AgentChatSlice/components/AgentTranscript.tsx +++ b/web/oss/src/components/AgentChatSlice/components/AgentTranscript.tsx @@ -100,9 +100,7 @@ const AgentTranscript = ({ )} {(!useVirtuoso || messages.length === 0) && (
{ - scroll.scrollRef.current = el - }} + ref={scroll.attachScroll} onScroll={scroll.onScroll} // Capture a fresh SC-3 anchor before a click acts (expand/collapse a tool step, // reasoning fold): those resize the transcript without a scroll, so onScroll never diff --git a/web/oss/src/components/AgentChatSlice/hooks/useComposerAttachments.ts b/web/oss/src/components/AgentChatSlice/hooks/useComposerAttachments.ts index 39bfaa7e9b..373c0afa82 100644 --- a/web/oss/src/components/AgentChatSlice/hooks/useComposerAttachments.ts +++ b/web/oss/src/components/AgentChatSlice/hooks/useComposerAttachments.ts @@ -1,5 +1,6 @@ import {useEffect, useRef, useState} from "react" +import {generateId} from "@agenta/shared/utils" import type {UploadFile} from "antd" import { @@ -12,8 +13,11 @@ import {attachmentsBySession} from "../state/sessionEphemera" import {useAttachmentUploads} from "./useAttachmentUploads" +// `uid` is the tray's React key, its preview-URL key, and the remove / view / retry handle, so it +// has to be unique per TRAY ROW. Deriving it from name+mtime+size collided whenever the same file +// was attached twice (paste then drop) — one remove then wiped both rows and their previews. const toUploadFile = (file: File): UploadFile => ({ - uid: `${file.name}-${file.lastModified}-${file.size}`, + uid: `att-${generateId()}`, name: file.name, status: "done", originFileObj: file as UploadFile["originFileObj"], @@ -62,8 +66,11 @@ export const useComposerAttachments = ({sessionId}: {sessionId: string}) => { const {accepted, rejections} = validateIncoming(incoming, files.length, limits) const allRejections = [...extraRejections, ...rejections] if (accepted.length) { - setFiles((prev) => [...prev, ...accepted.map(toUploadFile)]) - uploads.enqueue(accepted.map((f) => `${f.name}-${f.lastModified}-${f.size}`)) + // Stage once and enqueue the MINTED uids — re-deriving them here is what let the tray + // row and its upload disagree about which entry they addressed. + const staged = accepted.map(toUploadFile) + setFiles((prev) => [...prev, ...staged]) + uploads.enqueue(staged.map((f) => f.uid)) } setRejections(allRejections) // Open for rejections too. Otherwise dropping something unsupported writes a message into diff --git a/web/oss/src/components/AgentChatSlice/hooks/useOnboardingChat.ts b/web/oss/src/components/AgentChatSlice/hooks/useOnboardingChat.ts index 7a809499be..5dd6506c91 100644 --- a/web/oss/src/components/AgentChatSlice/hooks/useOnboardingChat.ts +++ b/web/oss/src/components/AgentChatSlice/hooks/useOnboardingChat.ts @@ -180,7 +180,14 @@ export const useOnboardingChat = ({ // Holds the pending IDE-bubble typewriter timer so it can be cancelled on unmount (tab close, // rewind, route change) — otherwise the recursive chain keeps calling setMessages on a stale closure. const ideBubbleTimerRef = useRef(null) + // The ref holds ONE handle, so anything that starts (or invalidates) a chain must cancel the + // previous one first — overwriting the handle would strand the old chain beyond every cleanup. + const cancelIdeBubble = useCallback(() => { + if (ideBubbleTimerRef.current) window.clearTimeout(ideBubbleTimerRef.current) + ideBubbleTimerRef.current = null + }, []) const streamIdeBubble = useCallback(() => { + cancelIdeBubble() const prompt = richInputRef.current?.getMarkdown().trim() ?? "" const promptQuote = prompt .split("\n") @@ -219,23 +226,21 @@ export const useOnboardingChat = ({ if (shown < full.length) ideBubbleTimerRef.current = window.setTimeout(tick, 28) } ideBubbleTimerRef.current = window.setTimeout(tick, 120) - }, [setMessages]) + }, [setMessages, cancelIdeBubble]) // Cancel any in-flight IDE-bubble animation on unmount so its timer chain can't fire post-unmount. - useEffect( - () => () => { - if (ideBubbleTimerRef.current) window.clearTimeout(ideBubbleTimerRef.current) - }, - [], - ) + useEffect(() => cancelIdeBubble, [cancelIdeBubble]) // After an IDE hand-off (onboarding + messages exist but nothing was committed), the chat is a // dead-end — there's no agent to talk to. Disable the composer and offer a single "Start over". const ideHandoffActive = onboardingActive && messages.length > 0 const handleStartOver = useCallback(() => { + // Start over wipes the transcript the chain is typing into — stop it, or it keeps ticking + // against messages that no longer exist (and outlives the next chain's handle). + cancelIdeBubble() setMessages(() => []) richInputRef.current?.setMarkdown("") - }, [setMessages]) + }, [setMessages, cancelIdeBubble]) // Strip era (TEMPLATE_STRIP_MODE): the bare "what do you want to build?" hero (no messages yet, // nothing pending, not browsing the template gallery) is when the onboarding TemplateStrip docks diff --git a/web/oss/src/components/AgentChatSlice/hooks/useSessionHydration.ts b/web/oss/src/components/AgentChatSlice/hooks/useSessionHydration.ts index ae7be37080..92815dfdf0 100644 --- a/web/oss/src/components/AgentChatSlice/hooks/useSessionHydration.ts +++ b/web/oss/src/components/AgentChatSlice/hooks/useSessionHydration.ts @@ -1,4 +1,4 @@ -import {type MutableRefObject, useCallback, useEffect, useState} from "react" +import {type MutableRefObject, useCallback, useEffect, useRef, useState} from "react" import {shouldAdoptServerTranscript} from "@agenta/entities/session" import {type UIMessage} from "ai" @@ -95,11 +95,20 @@ export const useSessionHydration = ({ // yank a reader who scrolled up. Following the growth is `stickRef`'s call, the same // rule the live stream uses. if (armJump || intent.stickRef.current) intent.armJump() + // Written synchronously, before any React commit. `messagesRef` lags a commit behind, + // so two deliveries landing back-to-back (disk-restored result + background refetch) + // can both see the pre-adoption transcript — it is this watermark, not the on-screen + // length, that keeps the guard order-independent and stops an older snapshot from + // clobbering a newer one. recordWatermarkRef.current = recordCount setMessages(serverMsgs) persistMessages({id: sessionId, messages: serverMsgs, recordCount}) return true }, + // `intent`'s MEMBERS, not `intent`: `useScrollIntent` returns a fresh object every render, + // so the object itself would recreate this callback each render and churn everything keyed + // on it. `armJump` (useCallback []) and `stickRef` (useRef) are stable for the life of the + // conversation. [ sessionId, messagesRef, @@ -109,9 +118,16 @@ export const useSessionHydration = ({ recordWatermarkRef, setMessages, persistMessages, - intent, + intent.armJump, + intent.stickRef, ], ) + // The remote-run poll below must NOT re-arm its timer on re-renders: the liveness query alone + // re-renders this hook ~every 15s while a run is live elsewhere, and restarting a fresh 15s + // timer on each of those starves the poll and resets its backoff. The effect reads the CURRENT + // adopter through this ref and keys only on the poll's real inputs. + const adoptServerTranscriptRef = useRef(adoptServerTranscript) + adoptServerTranscriptRef.current = adoptServerTranscript useEffect(() => { // A session created brand-new in this browser and not yet run has no backend records — @@ -125,18 +141,27 @@ export const useSessionHydration = ({ // instead of latching a ref that leaves the transcript blank. let cancelled = false // Post-restore revalidation: the first result may be the disk-restored log (paints - // instantly); adopt the background refetch when it lands. + // instantly); adopt the background refetch when it lands. The refetch can land BEFORE the + // promise handler below runs (both are microtasks racing), and `messagesRef` only catches + // up on the next React commit — so record here, not via what's on screen, that real + // history was already adopted. + let adopted = false loadSessionMessages(sessionId, (fresh) => { if (cancelled) return // The restore said "no records" but the server has some — clear the notice. - if (adoptServerTranscript(fresh)) setHydratedEmpty(false) + if (adoptServerTranscript(fresh)) { + adopted = true + setHydratedEmpty(false) + } }) .then((transcript) => { if (cancelled) return if (!transcript || transcript.messages.length === 0) { // Known session, but the server has no records for it → history was pruned or // never persisted. Flag it so the transcript shows the "unavailable" notice. - setHydratedEmpty(true) + // Only when nothing has been adopted yet — a refetch that already landed is + // real history, and this stale first result must not blank it out. + if (!adopted) setHydratedEmpty(true) return } adoptServerTranscript(transcript) @@ -193,10 +218,11 @@ export const useSessionHydration = ({ const poll = async () => { let grew = false try { + const adopt = adoptServerTranscriptRef.current const transcript = await loadSessionMessages(sessionId, (fresh) => { - if (!cancelled && adoptServerTranscript(fresh, {armJump: false})) grew = true + if (!cancelled && adopt(fresh, {armJump: false})) grew = true }) - if (!cancelled && adoptServerTranscript(transcript, {armJump: false})) grew = true + if (!cancelled && adopt(transcript, {armJump: false})) grew = true } catch { // `loadSessionMessages` already swallows + logs; keep polling regardless. } finally { @@ -213,7 +239,9 @@ export const useSessionHydration = ({ cancelled = true if (timer) clearTimeout(timer) } - }, [runningElsewhere, sessionId, adoptServerTranscript]) + // Deliberately NOT keyed on `adoptServerTranscript` — the poll reads it through the ref + // above, so a re-render can't cancel a pending tick or reset the backoff. + }, [runningElsewhere, sessionId]) return {isHydrating, hydratedEmpty, runningElsewhere} } diff --git a/web/oss/src/components/AgentChatSlice/hooks/useTranscriptScroll.ts b/web/oss/src/components/AgentChatSlice/hooks/useTranscriptScroll.ts index b681f3479c..a1f4128a01 100644 --- a/web/oss/src/components/AgentChatSlice/hooks/useTranscriptScroll.ts +++ b/web/oss/src/components/AgentChatSlice/hooks/useTranscriptScroll.ts @@ -1,4 +1,4 @@ -import {useCallback, useEffect, useLayoutEffect, useRef} from "react" +import {useCallback, useEffect, useLayoutEffect, useRef, useState} from "react" import {type ChatStatus, type UIMessage} from "ai" @@ -25,7 +25,16 @@ export const useTranscriptScroll = ({ useVirtuoso: boolean }) => { const {stickRef, armBottomRef, animateBottomRef, programmaticScrollRef, setShowJump} = intent - const scrollRef = useRef(null) + // The container is conditionally rendered (Virtuoso replaces it, and an empty conversation + // renders it even under Virtuoso), so it can unmount and remount within one session. The ref is + // for synchronous reads inside handlers; the STATE is what effects that bind listeners to the + // node key on — otherwise they'd stay attached to a node that has since been detached. + const scrollRef = useRef(null) + const [scrollNode, setScrollNode] = useState(null) + const attachScroll = useCallback((el: HTMLDivElement | null) => { + scrollRef.current = el + setScrollNode(el) + }, []) // Teardown for the in-flight smooth scroll (removes its listeners + fallback timer). const pinCleanupRef = useRef<(() => void) | null>(null) // Last observed scrollTop. A content shrink (tool gutter collapsing, reasoning folding) clamps @@ -58,6 +67,17 @@ export const useTranscriptScroll = ({ anchorRef.current = null }, []) + // Everything above is measured AGAINST a specific container, so a node swap invalidates all of + // it: the scroll baseline (a stale one makes the first scroll-down-to-edge on the new node fail + // `scrollTop > prevTop`, so follow doesn't re-arm), the SC-3 anchor (its offset was taken in the + // old node's coordinate space), and any glide still animating the node that just went away. + // Runs before the SC-1/SC-2 pin below, which is declared later. + useLayoutEffect(() => { + pinCleanupRef.current?.() + anchorRef.current = null + lastScrollTopRef.current = scrollNode?.scrollTop ?? 0 + }, [scrollNode]) + // ── DT4 autoscroll: stick to the bottom of the scrollable area while following ── // The fill (min-h-full turn group) makes "question at top" the scroll bottom for a short answer // and the answer's end the bottom for a long one, so scrollHeight is the right target (+ pb-6 gap). @@ -191,7 +211,7 @@ export const useTranscriptScroll = ({ // pins. Re-subscribed when the message set changes (a part growing fires on the same wrapper). useEffect(() => { if (useVirtuoso) return - const el = scrollRef.current + const el = scrollNode if (!el) return const onResize = (entries: ResizeObserverEntry[]) => { // Pin each rendered row's REAL height as its own `content-visibility` placeholder, so it @@ -247,7 +267,7 @@ export const useTranscriptScroll = ({ ro.observe(el) el.querySelectorAll("[data-mid]").forEach((w) => ro.observe(w)) return () => ro.disconnect() - }, [messages.length, scrollToBottom, useVirtuoso]) + }, [messages.length, scrollToBottom, useVirtuoso, scrollNode]) // SC-1 (submit) / SC-2 (restore): scroll the log to the bottom, once, when armed. With the active // turn reserving a viewport (min-h-full + top padding to clear the fade), "bottom" shows the new @@ -289,8 +309,10 @@ export const useTranscriptScroll = ({ // (exactly like a scroll). New content keeps arriving offscreen and the jump pill offers the way // back. Keyboard / wheel / touch already release because they scroll (onScroll). The composer is // exempt: its selections and links aren't inside the log, so `el.contains(...)` ignores them. + // Keyed on the NODE, not on mount: the container remounts when the engine changes, and a + // mount-only binding would keep listening on the detached one for the rest of the session. useEffect(() => { - const el = scrollRef.current + const el = scrollNode if (!el) return const release = () => { if (!stickRef.current) return @@ -312,7 +334,9 @@ export const useTranscriptScroll = ({ document.removeEventListener("selectionchange", onSelectionChange) el.removeEventListener("click", onClick) } - }, []) + }, [scrollNode]) - return {scrollRef, onScroll, recordAnchor, jumpToLatest} + // `attachScroll` is the ONLY way in: handing out `scrollRef` too would let a caller attach the + // node without the state ever knowing, which is the split this hook exists to close. + return {attachScroll, onScroll, recordAnchor, jumpToLatest} } diff --git a/web/oss/src/components/AgentChatSlice/state/sessions.ts b/web/oss/src/components/AgentChatSlice/state/sessions.ts index 1adef44374..13adf06d4b 100644 --- a/web/oss/src/components/AgentChatSlice/state/sessions.ts +++ b/web/oss/src/components/AgentChatSlice/state/sessions.ts @@ -668,13 +668,13 @@ const writeMessagesWithQuotaGuard = ( set: Setter, next: Record, keepId: string, -): string[] => { +): {evicted: string[]; persisted: boolean} => { let candidate = next const evicted: string[] = [] for (;;) { try { set(sessionMessagesAtom, candidate) - return evicted + return {evicted, persisted: true} } catch (e) { if (!isQuotaExceeded(e)) throw e // Object keys keep insertion order, so the first non-active id is the oldest. @@ -682,7 +682,7 @@ const writeMessagesWithQuotaGuard = ( if (oldest === undefined) { // Even the active session alone won't fit — keep it in memory, skip persistence. console.warn("[agent-chat] message store over quota; skipping persistence") - return evicted + return {evicted, persisted: false} } evicted.push(oldest) candidate = {...candidate} @@ -729,13 +729,18 @@ export const persistSessionMessagesAtom = atom( set, {id, messages, recordCount}: {id: string; messages: UIMessage[]; recordCount?: number}, ) => { - const evicted = writeMessagesWithQuotaGuard( + const {evicted, persisted} = writeMessagesWithQuotaGuard( set, {...get(sessionMessagesAtom), [id]: messages}, id, ) const counts = {...get(sessionRecordCountsAtom)} - if (recordCount === undefined) delete counts[id] + // If the transcript write itself was skipped (a single session over quota), the persisted + // store still holds the OLD messages — filing the NEW watermark against them would make + // `shouldAdoptServerTranscript` reject the complete server log as "not newer" on every + // future open, freezing the stale cache. The stores must never diverge (see + // `dropSessionMessages`), so drop the watermark and let the next open re-sync. + if (!persisted || recordCount === undefined) delete counts[id] else counts[id] = recordCount // A quota eviction dropped those transcripts, so their watermarks go too. for (const evictedId of evicted) delete counts[evictedId]