[fix] Rebase #5589 onto #5569 and fix the two blocking review findings

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.

web/oss/src/components/AgentChatSlice/assets/files.ts

@@ -32,9 +32,29 @@ const fileToPart = (file: File): Promise<FileUIPart> =>
         reader.readAsDataURL(file)
     })
 
-/** Convert picked `File`s into `file` parts for `sendMessage({text, files})`. */
-export const filesToParts = (files: File[]): Promise<FileUIPart[]> =>
-    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<FileReadResult> => {
+    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[] =>

web/oss/src/components/AgentChatSlice/components/AgentTranscript.tsx

@@ -100,9 +100,7 @@ const AgentTranscript = ({
             )}
             {(!useVirtuoso || messages.length === 0) && (
                 <div
-                    ref={(el) => {
-                        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

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

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<number | null>(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

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}
 }

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<HTMLDivElement>(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<HTMLDivElement | null>(null)
+    const [scrollNode, setScrollNode] = useState<HTMLDivElement | null>(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}
 }