diff --git a/apps/web/src/components/app/chat/chat-composer-attachments.test.tsx b/apps/web/src/components/app/chat/chat-composer-attachments.test.tsx index 6eacaf78..43aa2a0f 100644 --- a/apps/web/src/components/app/chat/chat-composer-attachments.test.tsx +++ b/apps/web/src/components/app/chat/chat-composer-attachments.test.tsx @@ -10,7 +10,6 @@ import { import { afterEach, beforeEach, describe, expect, it, vi } from "vitest"; import { ChatComposer } from "@/components/app/chat/chat-composer"; -import type { AgentPin } from "@/components/app/types"; import { isLongPaste, @@ -37,18 +36,6 @@ beforeEach(() => { }); }); -const pins: AgentPin[] = [ - { - id: "pin-1", - label: "Dev URL", - value: "http://localhost:5173", - type: "url", - }, - { id: "pin-2", label: "Branch", value: "feat/x", type: "string" }, - { label: "No id", value: "x", type: "string" }, - { id: "pin-3", label: "Run it", value: "go", type: "shortcut" }, -]; - function renderComposer( props: Partial[0]> = {} ) { @@ -63,7 +50,6 @@ function renderComposer( agentId={null} onSend={onSend} uploadFile={uploadFile} - pins={pins} disabledReason={null} {...props} /> @@ -157,25 +143,19 @@ describe("ChatComposer attachments", () => { ]); pasteText(input, "https://example.com/x"); pasteText(input, "line\n".repeat(90)); - fireEvent.click(screen.getByTestId("chat-composer-pin-button")); - fireEvent.click( - (await screen.findAllByTestId("chat-composer-pin-option"))[0]! - ); fireEvent.change(input, { target: { value: "everything" } }); expect(screen.getAllByTestId("context-file-item")).toHaveLength(2); expect(screen.getByTestId("chat-attachment-chip-pasted")).toBeTruthy(); expect(screen.getByTestId("context-link-item")).toBeTruthy(); - expect(screen.getByTestId("chat-attachment-chip-pin")).toBeTruthy(); fireEvent.keyDown(input, { key: "Enter" }); await waitFor(() => expect(onSend).toHaveBeenCalledTimes(1)); - expect(onSend.mock.calls[0]![1]).toHaveLength(5); + expect(onSend.mock.calls[0]![1]).toHaveLength(4); await waitFor(() => expect(input.value).toBe("")); expect(screen.queryByTestId("chat-composer-attachments")).toBeNull(); expect(screen.queryAllByTestId("context-file-item")).toHaveLength(0); expect(screen.queryByTestId("chat-attachment-chip-pasted")).toBeNull(); expect(screen.queryByTestId("context-link-item")).toBeNull(); - expect(screen.queryByTestId("chat-attachment-chip-pin")).toBeNull(); expect(screen.queryByTestId("chat-attachment-chip-placeholder")).toBeNull(); expect(screen.queryByTestId("chat-composer-error")).toBeNull(); // The image's object URL went with its chip. @@ -276,28 +256,16 @@ describe("ChatComposer attachments", () => { ).toBe(true); }); - it("attaches a pin from the picker, skipping pins without ids and shortcuts", async () => { + it("offers no way to attach a pin: files and links are the user's kinds", () => { renderComposer(); - fireEvent.click(screen.getByTestId("chat-composer-pin-button")); - const options = await screen.findAllByTestId("chat-composer-pin-option"); - expect(options.map((o) => o.getAttribute("data-pin-id"))).toEqual([ - "pin-1", - "pin-2", - ]); - fireEvent.click(options[0]!); - const chip = screen.getByTestId("chat-attachment-chip-pin"); - expect(chip.textContent).toContain("Dev URL"); - expect(chip.textContent).toContain("http://localhost:5173"); + expect(screen.queryByTestId("chat-composer-pin-button")).toBeNull(); + expect(screen.queryByLabelText("Attach a pin")).toBeNull(); }); it("uploads files on send and sends every attachment kind", async () => { const { onSend, uploadFile, input } = renderComposer(); pasteFiles(input, [new File(["png"], "shot.png", { type: "image/png" })]); pasteText(input, "https://example.com/x"); - fireEvent.click(screen.getByTestId("chat-composer-pin-button")); - fireEvent.click( - (await screen.findAllByTestId("chat-composer-pin-option"))[1]! - ); fireEvent.change(input, { target: { value: "look at these" } }); fireEvent.keyDown(input, { key: "Enter" }); @@ -307,7 +275,6 @@ describe("ChatComposer attachments", () => { expect(onSend).toHaveBeenCalledWith("look at these", [ { type: "file", mediaId: "shot.png".length }, { type: "link", url: "https://example.com/x" }, - { type: "pin", pinId: "pin-2" }, ]); await waitFor(() => expect(input.value).toBe("")); expect(screen.queryByTestId("chat-composer-attachments")).toBeNull(); @@ -419,15 +386,11 @@ describe("ChatComposer attachments", () => { ).toBe("drop.md"); }); - it("disables the attach buttons with the composer", () => { + it("disables the attach button with the composer", () => { renderComposer({ disabledReason: "The agent is not running." }); expect( (screen.getByTestId("chat-composer-attach-button") as HTMLButtonElement) .disabled ).toBe(true); - expect( - (screen.getByTestId("chat-composer-pin-button") as HTMLButtonElement) - .disabled - ).toBe(true); }); }); diff --git a/apps/web/src/components/app/chat/chat-composer-attachments.tsx b/apps/web/src/components/app/chat/chat-composer-attachments.tsx index 76474467..dea098d0 100644 --- a/apps/web/src/components/app/chat/chat-composer-attachments.tsx +++ b/apps/web/src/components/app/chat/chat-composer-attachments.tsx @@ -1,42 +1,9 @@ -import { useState } from "react"; -import { FileText, Paperclip, Pin } from "lucide-react"; +import { FileText, Paperclip } from "lucide-react"; import { ContextChip } from "@/components/app/context-picker-items"; -import { type AgentPin } from "@/components/app/types"; import { type ChatDraftFile } from "@/lib/chat-draft"; -import { Button } from "@/components/ui/button"; -import { - Popover, - PopoverContent, - PopoverTrigger, -} from "@/components/ui/popover"; import { cn } from "@/lib/utils"; -/** Pins the composer can attach: addressable by id, and not a button. */ -export function attachablePins(pins: AgentPin[]): AgentPin[] { - return pins.filter((pin) => !!pin.id && pin.type !== "shortcut"); -} - -export function PinChip({ - pin, - onRemove, -}: { - pin: AgentPin; - onRemove: () => void; -}): JSX.Element { - return ( - } - title={pin.label} - subtitle={pin.value} - onRemove={onRemove} - removeLabel={`Remove pin ${pin.label}`} - tooltip={`${pin.label}: ${pin.value}`} - testId="chat-attachment-chip-pin" - /> - ); -} - /** The `pasted.txt` chip: a long paste turned into a file, with a way back. */ export function PastedTextChip({ file, @@ -116,86 +83,3 @@ export function DraftPlaceholderChip({ /> ); } - -/** - * The paperclip's sibling: a popover listing the agent's pins so one can ride - * along with the message. Already-attached pins stay listed but inert. - */ -export function PinPickerButton({ - pins, - attachedIds, - disabled, - onPick, -}: { - pins: AgentPin[]; - attachedIds: ReadonlySet; - disabled: boolean; - onPick: (pin: AgentPin) => void; -}): JSX.Element { - const [open, setOpen] = useState(false); - const candidates = attachablePins(pins); - return ( - - - - - - {candidates.length === 0 ? ( -
- No pins to attach yet. -
- ) : ( -
- {candidates.map((pin) => { - const attached = attachedIds.has(pin.id!); - return ( - - ); - })} -
- )} - - - ); -} diff --git a/apps/web/src/components/app/chat/chat-composer-draft.test.tsx b/apps/web/src/components/app/chat/chat-composer-draft.test.tsx index 6077a305..1cc462b4 100644 --- a/apps/web/src/components/app/chat/chat-composer-draft.test.tsx +++ b/apps/web/src/components/app/chat/chat-composer-draft.test.tsx @@ -10,7 +10,6 @@ import { import { afterEach, beforeEach, describe, expect, it, vi } from "vitest"; import { ChatComposer } from "@/components/app/chat/chat-composer"; -import type { AgentPin } from "@/components/app/types"; import { CHAT_DRAFT_MAX_BYTES, type ChatComposerDraft, @@ -18,15 +17,6 @@ import { } from "@/lib/chat-draft"; import { CHAT_DRAFT_STORAGE_PREFIX, chatDraftAtomFamily } from "@/lib/store"; -const pins: AgentPin[] = [ - { - id: "pin-1", - label: "Dev URL", - value: "http://localhost:5173", - type: "url", - }, -]; - const usedAgentIds = new Set(); afterEach(() => { @@ -84,7 +74,6 @@ function renderComposer( agentId={agentId} onSend={onSend} uploadFile={uploadFile} - pins={pins} disabledReason={null} {...props} /> @@ -162,13 +151,45 @@ describe("ChatComposer draft persistence", () => { expect(writes.every((draft) => draft.files.length === 0)).toBe(true); }); + it("stays put after sending a file: one write clears it and no chip comes back", async () => { + // The regression: with local file state mirrored into the draft by + // effects, a send left the two disagreeing for a render and the chip + // flickered back (and typing then cleared it). Now the draft is the + // only source of what is attached, so the send is one write and the + // chips after it are whatever that write says — nothing. + const { onSend, input } = renderComposer("agt_stable"); + pasteFiles(input, [new File(["png"], "shot.png", { type: "image/png" })]); + fireEvent.change(input, { target: { value: "look" } }); + expect(screen.getByTestId("context-file-item")).toBeTruthy(); + + const setItem = vi.spyOn(Storage.prototype, "setItem"); + fireEvent.keyDown(input, { key: "Enter" }); + await waitFor(() => expect(onSend).toHaveBeenCalledTimes(1)); + await waitFor(() => expect(input.value).toBe("")); + const writes = () => + setItem.mock.calls.filter(([key]) => key === storageKey("agt_stable")); + expect(writes()).toHaveLength(1); + expect(stored("agt_stable")).toEqual(EMPTY_CHAT_DRAFT); + expect(chipNames()).toEqual([]); + expect(screen.queryByTestId("chat-composer-attachments")).toBeNull(); + + // Further renders — typing, a tick — change nothing about the chips, + // and typing is a write about the text alone. + fireEvent.change(input, { target: { value: "next" } }); + await act(async () => {}); + expect(chipNames()).toEqual([]); + expect(writes()).toHaveLength(2); + expect(stored("agt_stable")).toEqual({ ...EMPTY_CHAT_DRAFT, text: "next" }); + // The image's object URL went with its chip, once. + expect(URL.revokeObjectURL).toHaveBeenCalledTimes(1); + }); + it("does not echo its copy of the draft back to storage when another tab's write only reshapes local chips", () => { renderComposer("agt_echo"); const setItem = vi.spyOn(Storage.prototype, "setItem"); otherTabWrites("agt_echo", { text: "typed over there", links: [], - pinIds: [], files: [{ name: "shot.png", size: 3, mime: "image/png" }], }); // The file becomes a placeholder here... @@ -183,35 +204,65 @@ describe("ChatComposer draft persistence", () => { ).toHaveLength(1); }); - it("persists the text, links and pins as they are added", () => { + it("persists the text and links as they are added", () => { const { input } = renderComposer("agt_persist"); fireEvent.change(input, { target: { value: "half a thought" } }); expect(stored("agt_persist").text).toBe("half a thought"); pasteText(input, "https://example.com/spec"); expect(stored("agt_persist").links).toEqual(["https://example.com/spec"]); - - fireEvent.click(screen.getByTestId("chat-composer-pin-button")); - fireEvent.click(screen.getByTestId("chat-composer-pin-option")); - expect(stored("agt_persist").pinIds).toEqual(["pin-1"]); + expect(stored("agt_persist")).toEqual({ + text: "half a thought", + links: ["https://example.com/spec"], + files: [], + }); }); - it("restores the text, link and pin chips on mount", () => { + it("restores the text and link chips on mount", () => { seed("agt_restore", { text: "still here", links: ["https://example.com/a"], - pinIds: ["pin-1", "pin-gone"], }); const { input } = renderComposer("agt_restore"); expect(input.value).toBe("still here"); expect(screen.getByTestId("context-link-item").getAttribute("title")).toBe( "https://example.com/a" ); - // A pin the agent no longer has is not shown and not sent. - expect(screen.getAllByTestId("chat-attachment-chip-pin")).toHaveLength(1); expect(sendButton().disabled).toBe(false); }); + it("restores a legacy draft that carried pinIds, ignoring them", async () => { + // Drafts written before the pin picker was dropped: the field must not + // fail validation, bring back a chip, be sent, or be written back. + usedAgentIds.add("agt_legacy"); + window.localStorage.setItem( + storageKey("agt_legacy"), + JSON.stringify({ + text: "from before", + links: ["https://example.com/a"], + pinIds: ["pin-1", "pin-gone"], + files: [], + }) + ); + const { input, onSend } = renderComposer("agt_legacy"); + expect(input.value).toBe("from before"); + expect(screen.getByTestId("context-link-item")).toBeTruthy(); + expect(screen.queryByTestId("chat-attachment-chip-pin")).toBeNull(); + + fireEvent.change(input, { target: { value: "from before, edited" } }); + expect(stored("agt_legacy")).toEqual({ + text: "from before, edited", + links: ["https://example.com/a"], + files: [], + }); + + fireEvent.keyDown(input, { key: "Enter" }); + await waitFor(() => expect(onSend).toHaveBeenCalledTimes(1)); + expect(onSend).toHaveBeenCalledWith("from before, edited", [ + { type: "link", url: "https://example.com/a" }, + ]); + }); + it("keeps drafts apart per agent", () => { seed("agt_one", { text: "for one" }); seed("agt_two", { text: "for two" }); @@ -286,15 +337,16 @@ describe("ChatComposer draft persistence", () => { expect(sendButton().disabled).toBe(true); pasteFiles(input, [new File(["png"], "shot.png", { type: "image/png" })]); - // One placeholder consumed, the other still holds the send. + // One placeholder consumed — the file takes its slot, in its place — the + // other still holds the send. expect( screen.getAllByTestId("chat-attachment-chip-placeholder") ).toHaveLength(1); expect(screen.getAllByTestId("context-file-item")).toHaveLength(1); expect(sendButton().disabled).toBe(true); expect(stored("agt_reattach").files).toEqual([ - { name: "notes.txt", size: 5, mime: "text/plain" }, { name: "shot.png", size: 3, mime: "image/png" }, + { name: "notes.txt", size: 5, mime: "text/plain" }, ]); // A different file of the same name is not the one that was attached. @@ -394,13 +446,11 @@ describe("ChatComposer draft persistence", () => { seed("agt_clear", { text: "ship it", links: ["https://example.com/pr"], - pinIds: ["pin-1"], }); const { input } = renderComposer("agt_clear", { onSend }); fireEvent.keyDown(input, { key: "Enter" }); expect(onSend).toHaveBeenCalledWith("ship it", [ { type: "link", url: "https://example.com/pr" }, - { type: "pin", pinId: "pin-1" }, ]); fireEvent.change(input, { target: { value: "next thought" } }); @@ -413,7 +463,6 @@ describe("ChatComposer draft persistence", () => { text: "next thought", }); expect(screen.queryByTestId("context-link-item")).toBeNull(); - expect(screen.queryByTestId("chat-attachment-chip-pin")).toBeNull(); }); it("keeps the draft on a failed send", async () => { @@ -496,8 +545,8 @@ describe("ChatComposer draft persistence", () => { screen.getByTestId("chat-attachment-chip-pasted").textContent ).toContain("90 lines"); expect(sendButton().disabled).toBe(true); - // This tab's description of the draft matches what was written, in - // its order, so nothing bounces back to storage. + // The chips are the other tab's list, in its order; this tab wrote + // nothing back. expect(stored("agt_tabs").files.map((f) => f.name)).toEqual([ "notes.pdf", "shot.png", diff --git a/apps/web/src/components/app/chat/chat-composer.tsx b/apps/web/src/components/app/chat/chat-composer.tsx index 64ba905f..5c8c123e 100644 --- a/apps/web/src/components/app/chat/chat-composer.tsx +++ b/apps/web/src/components/app/chat/chat-composer.tsx @@ -25,8 +25,6 @@ import { import { DraftPlaceholderChip, PastedTextChip, - PinChip, - PinPickerButton, } from "@/components/app/chat/chat-composer-attachments"; import { ContextFileItem, @@ -35,16 +33,14 @@ import { import { STARTUP_FILE_ACCEPT, getClipboardFilesFromEvent, - startupFileKey, } from "@/components/app/create-agent-dialog-clipboard"; -import { type AgentPin } from "@/components/app/types"; import { Button } from "@/components/ui/button"; import { Textarea } from "@/components/ui/textarea"; import { type ChatComposerDraft, type ChatDraftFile, EMPTY_CHAT_DRAFT, - isChatComposerDraft, + readChatComposerDraft, } from "@/lib/chat-draft"; import { isImageFile } from "@/lib/media-accept"; import { isAcceptedUploadFile } from "@/lib/media-upload"; @@ -71,8 +67,6 @@ export type ChatComposerProps = { * time, once per file; a rejection keeps the draft and marks the chip. */ uploadFile?: (file: File) => Promise<{ id: number }>; - /** The agent's pins, for the pin picker. */ - pins?: AgentPin[]; /** When set, the composer is disabled and this explains why. */ disabledReason: string | null; /** An external send is in flight; the input stays usable, the button waits. */ @@ -91,10 +85,8 @@ export type ChatComposerProps = { replyContext?: { excerpt: string; onDismiss: () => void } | null; }; -const NO_PINS: AgentPin[] = []; - /** What is kept of a live file across a reload: its identity, and a paste's text. */ -function describeFile(file: File, pasted: string | undefined): ChatDraftFile { +function describeFile(file: File, pasted?: string): ChatDraftFile { return { name: file.name, size: file.size, @@ -103,6 +95,15 @@ function describeFile(file: File, pasted: string | undefined): ChatDraftFile { }; } +/** + * Identity of a draft file entry, and of the live `File` standing behind it + * (`describeFile` of the `File` gives the same key). Keys the in-memory + * bookkeeping: live files, previews, media ids, upload state. + */ +function draftFileKey(entry: ChatDraftFile): string { + return `${entry.name}:${entry.size}:${entry.mime}`; +} + /** * Whether a file gets a thumbnail. By name as well as by MIME type: a file * pasted from the clipboard can arrive as `image.png` with an empty `type` @@ -113,10 +114,6 @@ function isImageAttachment(file: File): boolean { return isImageFile(file.name) || file.type.startsWith("image/"); } -function sameFiles(a: ChatDraftFile[], b: ChatDraftFile[]): boolean { - return JSON.stringify(a) === JSON.stringify(b); -} - /** * A stored descriptor and a live file are the same attachment when name and * size agree — and the MIME type too, when both sides know it (a picked @@ -130,118 +127,13 @@ function describesFile(entry: ChatDraftFile, file: File): boolean { ); } -function sameDescriptor(a: ChatDraftFile, b: ChatDraftFile): boolean { - return ( - a.name === b.name && - a.size === b.size && - a.mime === b.mime && - (a.pasted ?? null) === (b.pasted ?? null) - ); -} - -function sameList(a: readonly T[], b: readonly T[]): boolean { - return a.length === b.length && a.every((item, i) => item === b[i]); -} - -/** - * Re-attaching a file that a placeholder stands for (after a reload, say) - * replaces the placeholder rather than sitting beside it. Returns the - * placeholders left over and how many the incoming files took. - */ -function consumePlaceholders( - placeholders: ChatDraftFile[], - incoming: File[] -): { remaining: ChatDraftFile[]; consumed: number } { - const remaining = [...placeholders]; - for (const file of incoming) { - const index = remaining.findIndex((entry) => describesFile(entry, file)); - if (index !== -1) remaining.splice(index, 1); - } - return { remaining, consumed: placeholders.length - remaining.length }; -} - -/** - * Brings this tab's live/placeholder model in line with the draft's file - * descriptors after they changed under it — another tab attached, removed - * or pasted something. Descriptors this tab has a `File` for (by name and - * size) keep that object; a pasted body it lacks becomes a live pasted-text - * chip; anything else becomes a placeholder; live files no descriptor - * mentions any more are dropped. Order follows the descriptors, so every - * tab converges on the same chip order. - */ -function reconcileDraftFiles( - stored: ChatDraftFile[], - live: File[], - placeholders: ChatDraftFile[] -): { - files: File[]; - placeholders: ChatDraftFile[]; - dropped: File[]; - restoredPasted: Map; - changed: boolean; -} { - const unusedLive = [...live]; - const unusedPlaceholders = [...placeholders]; - const files: File[] = []; - const nextPlaceholders: ChatDraftFile[] = []; - const restoredPasted = new Map(); - for (const entry of stored) { - const liveIndex = unusedLive.findIndex((file) => - describesFile(entry, file) - ); - if (liveIndex !== -1) { - files.push(unusedLive.splice(liveIndex, 1)[0]!); - continue; - } - if (typeof entry.pasted === "string") { - const file = pastedTextFile(entry.pasted, entry.name); - restoredPasted.set(startupFileKey(file), entry.pasted); - files.push(file); - continue; - } - const placeholderIndex = unusedPlaceholders.findIndex((candidate) => - sameDescriptor(candidate, entry) - ); - nextPlaceholders.push( - placeholderIndex === -1 - ? entry - : unusedPlaceholders.splice(placeholderIndex, 1)[0]! - ); - } - return { - files, - placeholders: nextPlaceholders, - dropped: unusedLive, - restoredPasted, - changed: - !sameList(files, live) || !sameList(nextPlaceholders, placeholders), - }; -} - -/** - * Splits a stored draft's files into what can come back live (a pasted text - * chip whose body was kept) and what can only be a placeholder (a picked - * file, or a paste whose body was dropped for size). - */ -function restoreDraftFiles(draft: ChatComposerDraft): { - files: File[]; - pasted: Map; - placeholders: ChatDraftFile[]; -} { - const files: File[] = []; - const pasted = new Map(); - const placeholders: ChatDraftFile[] = []; - for (const entry of draft.files) { - if (typeof entry.pasted === "string") { - const file = pastedTextFile(entry.pasted, entry.name); - pasted.set(startupFileKey(file), entry.pasted); - files.push(file); - } else { - placeholders.push(entry); - } - } - return { files, pasted, placeholders }; -} +/** One draft file as rendered: its entry, its key, and its live `File` if any. */ +type DraftFileView = { + entry: ChatDraftFile; + key: string; + /** Absent for a placeholder: a file this tab has no bytes for. */ + file: File | undefined; +}; /** * What went wrong, and whether pressing Enter again can help. A validation @@ -259,22 +151,24 @@ const SUPPORTED_FILE_HINT = * is left alone: the Enter that commits a CJK candidate must not send. * * Attachments ride along as chips above the field: files from the - * paperclip, a drop or a paste; links from a pasted URL; pins from the pin - * picker. Files upload when the message is sent, so an unsent draft leaves - * nothing behind on the server. + * paperclip, a drop or a paste; links from a pasted URL. Files upload when + * the message is sent, so an unsent draft leaves nothing behind on the + * server. * - * The draft — text, links, pins, pasted text — is persisted per agent - * (`chatDraftAtomFamily`) and comes back after a reload. Picked files - * cannot: they come back as "needs re-attaching" placeholders that hold the - * send until they are re-attached (which replaces the placeholder) or - * removed. Another tab editing the same draft shows up here too, files - * included — see `reconcileDraftFiles`. + * The draft — text, links, file descriptors, pasted text — is persisted per + * agent (`chatDraftAtomFamily`) and is the one source of truth for what is + * attached: chips render straight from `draft.files`. The bytes of a picked + * file live only in a ref, keyed by `draftFileKey`, and never round-trip + * through the draft. A descriptor this tab has no `File` for — after a + * reload, or written by another tab — renders as a "needs re-attaching" + * placeholder that holds the send until it is re-attached (which fills the + * same slot) or removed. Nothing here writes the draft from an effect: every + * change is one explicit write from the handler that caused it. */ export function ChatComposer({ agentId, onSend, uploadFile, - pins = NO_PINS, disabledReason, sending = false, placeholder = "Message the agent…", @@ -288,20 +182,16 @@ export function ChatComposer({ const [storedDraft, setStoredDraft] = useAtom( agentId ? chatDraftAtomFamily(agentId) : localDraftAtom ); - const draft = isChatComposerDraft(storedDraft) - ? storedDraft - : EMPTY_CHAT_DRAFT; + const draft = readChatComposerDraft(storedDraft); // The atom holds the draft in full; the size cap applies to what the atom // writes to storage (`chatDraftAtomFamily`), not to what is typed. const updateDraft = useCallback( (patch: (current: ChatComposerDraft) => ChatComposerDraft) => { - setStoredDraft((prev) => - patch(isChatComposerDraft(prev) ? prev : EMPTY_CHAT_DRAFT) - ); + setStoredDraft((prev) => patch(readChatComposerDraft(prev))); }, [setStoredDraft] ); - const { text, links, pinIds } = draft; + const { text, links } = draft; const setText = useCallback( (next: string | ((current: string) => string)) => { updateDraft((current) => ({ @@ -319,21 +209,13 @@ export function ChatComposer({ const disabled = disabledReason !== null; const trimmed = text.trim(); - // ---- files: live File objects, in memory only ----------------------------- - // Restored from the draft on mount. From then on the two are kept in step - // both ways: the draft's file list mirrors this state (the describe effect - // below), and a change to the draft's list from elsewhere — another tab, - // via the atom's storage subscription — is reconciled back into it. - const [restored] = useState(() => restoreDraftFiles(draft)); - const [files, setFiles] = useState(restored.files); - const [placeholders, setPlaceholders] = useState( - restored.placeholders - ); - // Per-file bookkeeping keyed by `startupFileKey`: the original text of a - // long paste (for "keep inline" and for persistence), the media id once - // uploaded (so a retry after a later failure does not upload it twice), - // image previews, and the upload state. - const pastedTextRef = useRef>(restored.pasted); + // ---- files: the draft has the descriptors, this ref has the bytes -------- + // Live `File` objects by `draftFileKey`. A pasted-text entry's file is + // rebuilt from the text in the draft on demand, so it is always live. + const filesRef = useRef>(new Map()); + // Per-file bookkeeping, same key: the media id once uploaded (so a retry + // after a later failure does not upload it twice), image previews, and + // the upload state. const mediaIdsRef = useRef>(new Map()); const previewsRef = useRef>(new Map()); const [fileStatus, setFileStatus] = useState< @@ -349,9 +231,17 @@ export function ChatComposer({ }; }, []); + /** Remembers a file's bytes for its entry, with a thumbnail for images. */ + const holdFile = useCallback((key: string, file: File) => { + filesRef.current.set(key, file); + if (isImageAttachment(file) && !previewsRef.current.has(key)) { + previewsRef.current.set(key, URL.createObjectURL(file)); + } + }, []); + /** Drops everything remembered about a file that is no longer attached. */ const forgetFile = useCallback((key: string) => { - pastedTextRef.current.delete(key); + filesRef.current.delete(key); mediaIdsRef.current.delete(key); const preview = previewsRef.current.get(key); if (preview) { @@ -366,75 +256,38 @@ export function ChatComposer({ }); }, []); - useEffect(() => { - const described = [ - ...placeholders, - ...files.map((file) => - describeFile(file, pastedTextRef.current.get(startupFileKey(file))) - ), - ]; - updateDraft((current) => - sameFiles(current.files, described) - ? current - : { ...current, files: described } - ); - }, [files, placeholders, updateDraft]); - - // The other direction. Keyed on the draft's list alone and reading the - // local model through refs, so it runs after a change *to the draft* — - // never after a local change, whose describe above has not landed yet — - // and is a no-op when the two already agree (which is the case right - // after every local change, and on mount). - const filesRef = useRef(files); - filesRef.current = files; - const placeholdersRef = useRef(placeholders); - placeholdersRef.current = placeholders; - useEffect(() => { - const next = reconcileDraftFiles( - draft.files, - filesRef.current, - placeholdersRef.current - ); - if (!next.changed) return; - for (const file of next.dropped) forgetFile(startupFileKey(file)); - for (const [key, text] of next.restoredPasted) { - pastedTextRef.current.set(key, text); - } - setFiles(next.files); - setPlaceholders(next.placeholders); - }, [draft.files, forgetFile]); - - const appendFiles = useCallback((incoming: File[]) => { - if (incoming.length === 0) return; - setPlaceholders((current) => { - const { remaining, consumed } = consumePlaceholders(current, incoming); - return consumed === 0 ? current : remaining; - }); - setFiles((current) => { - const next = [...current]; - const seen = new Set(current.map(startupFileKey)); - for (const file of incoming) { - const key = startupFileKey(file); - if (seen.has(key)) continue; - seen.add(key); - next.push(file); - if (isImageAttachment(file) && !previewsRef.current.has(key)) { - previewsRef.current.set(key, URL.createObjectURL(file)); - } - } - return next; - }); + /** The live file behind an entry, if this tab has (or can rebuild) one. */ + const fileFor = useCallback((entry: ChatDraftFile): File | undefined => { + const key = draftFileKey(entry); + const held = filesRef.current.get(key); + if (held) return held; + if (typeof entry.pasted !== "string") return undefined; + const file = pastedTextFile(entry.pasted, entry.name); + filesRef.current.set(key, file); + return file; }, []); - const attachedPins = useMemo( + const fileViews = useMemo( () => - pinIds - .map((id) => pins.find((pin) => pin.id === id)) - .filter((pin): pin is AgentPin => pin !== undefined), - [pinIds, pins] + draft.files.map((entry) => ({ + entry, + key: draftFileKey(entry), + file: fileFor(entry), + })), + [draft.files, fileFor] ); - const attachmentCount = - files.length + placeholders.length + links.length + attachedPins.length; + const placeholders = fileViews.filter((view) => view.file === undefined); + + // Bytes for entries the draft no longer lists (removed here, sent, or + // taken out by another tab) are let go. Reads the draft, never writes it. + useEffect(() => { + const listed = new Set(draft.files.map(draftFileKey)); + for (const key of [...filesRef.current.keys()]) { + if (!listed.has(key)) forgetFile(key); + } + }, [draft.files, forgetFile]); + + const attachmentCount = draft.files.length + links.length; const attachmentsFull = attachmentCount >= CHAT_ATTACHMENTS_MAX; const noteAttachmentLimit = useCallback(() => { @@ -462,16 +315,44 @@ export function ChatComposer({ } else { setError(null); } - // A file that re-attaches a placeholder takes its slot, not a new one. - const { consumed } = consumePlaceholders(placeholders, accepted); - const room = Math.max( - 0, - CHAT_ATTACHMENTS_MAX - attachmentCount + consumed - ); - if (accepted.length > room) noteAttachmentLimit(); - appendFiles(accepted.slice(0, room)); + if (accepted.length === 0) return; + // One write. A file that re-attaches a placeholder fills that slot — + // no new entry, no room needed; a file already attached is skipped; the + // rest append while there is room under the cap. + let overflowed = false; + const held: Array<[string, File]> = []; + updateDraft((current) => { + const files = [...current.files]; + let room = CHAT_ATTACHMENTS_MAX - files.length - current.links.length; + for (const file of accepted) { + const entry = describeFile(file); + const key = draftFileKey(entry); + if (filesRef.current.has(key)) continue; + const slot = files.findIndex( + (candidate) => + typeof candidate.pasted !== "string" && + !filesRef.current.has(draftFileKey(candidate)) && + describesFile(candidate, file) + ); + if (slot !== -1) { + files[slot] = entry; + held.push([key, file]); + continue; + } + if (room <= 0) { + overflowed = true; + continue; + } + room -= 1; + files.push(entry); + held.push([key, file]); + } + return held.length === 0 ? current : { ...current, files }; + }); + for (const [key, file] of held) holdFile(key, file); + if (overflowed) noteAttachmentLimit(); }, - [appendFiles, attachmentCount, noteAttachmentLimit, placeholders] + [holdFile, noteAttachmentLimit, updateDraft] ); const addLink = useCallback( @@ -500,48 +381,21 @@ export function ChatComposer({ [updateDraft] ); - const addPin = useCallback( - (pin: AgentPin) => { - if (!pin.id) return; - if (pinIds.includes(pin.id)) return; - if (attachmentsFull) { - noteAttachmentLimit(); - return; - } - updateDraft((current) => - current.pinIds.includes(pin.id!) + const removeEntry = useCallback( + (key: string) => { + updateDraft((current) => { + const files = current.files.filter( + (entry) => draftFileKey(entry) !== key + ); + return files.length === current.files.length ? current - : { ...current, pinIds: [...current.pinIds, pin.id!] } - ); - }, - [attachmentsFull, noteAttachmentLimit, pinIds, updateDraft] - ); - - const removePin = useCallback( - (pin: AgentPin) => { - updateDraft((current) => ({ - ...current, - pinIds: current.pinIds.filter((id) => id !== pin.id), - })); - }, - [updateDraft] - ); - - const removeFile = useCallback( - (file: File) => { - const key = startupFileKey(file); + : { ...current, files }; + }); forgetFile(key); - setFiles((current) => - current.filter((candidate) => startupFileKey(candidate) !== key) - ); }, - [forgetFile] + [forgetFile, updateDraft] ); - const removePlaceholder = useCallback((index: number) => { - setPlaceholders((current) => current.filter((_, i) => i !== index)); - }, []); - const addPastedText = useCallback( (pasted: string) => { if (attachmentsFull) { @@ -550,33 +404,40 @@ export function ChatComposer({ } const file = pastedTextFile( pasted, - nextPastedFileName([ - ...files.map((f) => f.name), - ...placeholders.map((f) => f.name), - ]) + nextPastedFileName(draft.files.map((entry) => entry.name)) ); - pastedTextRef.current.set(startupFileKey(file), pasted); + const entry = describeFile(file, pasted); + filesRef.current.set(draftFileKey(entry), file); setError(null); - appendFiles([file]); + updateDraft((current) => ({ + ...current, + files: [...current.files, entry], + })); return true; }, - [appendFiles, attachmentsFull, files, noteAttachmentLimit, placeholders] + [attachmentsFull, draft.files, noteAttachmentLimit, updateDraft] ); /** Undo for a long paste: drop the chip, put the text back in the field. */ const keepInline = useCallback( - (file: File) => { - const pasted = pastedTextRef.current.get(startupFileKey(file)) ?? ""; - removeFile(file); + (view: DraftFileView) => { + const pasted = view.entry.pasted ?? ""; const el = textareaRef.current; - setText((current) => { - const start = el?.selectionStart ?? current.length; - const end = el?.selectionEnd ?? current.length; - return current.slice(0, start) + pasted + current.slice(end); + updateDraft((current) => { + const start = el?.selectionStart ?? current.text.length; + const end = el?.selectionEnd ?? current.text.length; + return { + ...current, + text: current.text.slice(0, start) + pasted + current.text.slice(end), + files: current.files.filter( + (entry) => draftFileKey(entry) !== view.key + ), + }; }); + forgetFile(view.key); requestAnimationFrame(() => textareaRef.current?.focus()); }, - [removeFile, setText] + [forgetFile, updateDraft] ); const onPaste = useCallback( @@ -639,11 +500,6 @@ export function ChatComposer({ [addFiles, disabled] ); - const attachedPinIds = useMemo( - () => new Set(attachedPins.map((pin) => pin.id!)), - [attachedPins] - ); - const canSend = !disabled && !sending && @@ -670,14 +526,15 @@ export function ChatComposer({ // Only what was sent gets cleared: anything typed or attached while the // send was pending is a new draft and stays. const submittedText = text; - const submittedFiles = files; + const submittedFiles = fileViews; const submittedLinks = links; - const submittedPins = attachedPins; const run = async () => { const attachments: ChatUserAttachmentInput[] = []; - for (const file of submittedFiles) { - const key = startupFileKey(file); + for (const { entry, key, file } of submittedFiles) { + // `canSend` ruled out placeholders; this is the same check for the + // type system's sake. + if (!file) throw new Error(`Re-attach ${entry.name} to send.`); let mediaId = mediaIdsRef.current.get(key); if (mediaId === undefined) { if (!uploadFile) throw new Error("File uploads are not available."); @@ -702,28 +559,24 @@ export function ChatComposer({ attachments.push({ type: "file", mediaId }); } for (const url of submittedLinks) attachments.push({ type: "link", url }); - for (const pin of submittedPins) { - attachments.push({ type: "pin", pinId: pin.id! }); - } await onSend(submittedText.trim(), attachments); }; run() .then(() => { - const sentPinIds = new Set(submittedPins.map((pin) => pin.id)); - // The sent files leave the draft in this same write, not in the - // describe effect's follow-up: the draft is what a remounted - // composer or another tab restores from, and a draft that still - // listed them — even for one render — would bring them back as - // "needs re-attaching" placeholders. + // What was sent leaves the draft in one write — text, links and + // file entries together — so no render, remount or other tab ever + // sees a draft that still lists a sent file. + const sentKeys = new Set(submittedFiles.map((view) => view.key)); updateDraft((current) => ({ ...current, text: current.text === submittedText ? "" : current.text, links: current.links.filter((url) => !submittedLinks.includes(url)), - pinIds: current.pinIds.filter((id) => !sentPinIds.has(id)), - files: consumePlaceholders(current.files, submittedFiles).remaining, + files: current.files.filter( + (entry) => !sentKeys.has(draftFileKey(entry)) + ), })); - for (const file of submittedFiles) removeFile(file); + for (const key of sentKeys) forgetFile(key); }) .catch((err: unknown) => { // The draft — text and chips — is still here, so a retry can work. @@ -737,12 +590,11 @@ export function ChatComposer({ textareaRef.current?.focus(); }); }, [ - attachedPins, canSend, - files, + fileViews, + forgetFile, links, onSend, - removeFile, text, updateDraft, uploadFile, @@ -759,9 +611,9 @@ export function ChatComposer({ [submit] ); - const uploadingName = files.find( - (file) => fileStatus[startupFileKey(file)] === "uploading" - )?.name; + const uploadingName = fileViews.find( + (view) => fileStatus[view.key] === "uploading" + )?.entry.name; const hasAttachments = attachmentCount > 0; return ( @@ -820,35 +672,32 @@ export function ChatComposer({ className="flex max-h-40 flex-wrap items-start gap-3 overflow-y-auto px-3 pb-1 pt-3" data-testid="chat-composer-attachments" > - {placeholders.map((entry, index) => ( - removePlaceholder(index)} - /> - ))} - {files.map((file) => { - const key = startupFileKey(file); - const pasted = pastedTextRef.current.get(key); - return pasted !== undefined ? ( + {fileViews.map((view) => + view.file === undefined ? ( + removeEntry(view.key)} + /> + ) : typeof view.entry.pasted === "string" ? ( keepInline(file)} - onRemove={() => removeFile(file)} + key={view.key} + file={view.file} + lines={countLines(view.entry.pasted)} + status={fileStatus[view.key]} + onKeepInline={() => keepInline(view)} + onRemove={() => removeEntry(view.key)} /> ) : ( removeFile(file)} + key={view.key} + file={view.file} + preview={previewsRef.current.get(view.key)} + status={fileStatus[view.key]} + onRemove={() => removeEntry(view.key)} /> - ); - })} + ) + )} {links.map((link) => ( removeLink(link)} /> ))} - {attachedPins.map((pin) => ( - removePin(pin)} /> - ))} ) : null}
@@ -885,12 +731,6 @@ export function ChatComposer({ > -