From fd3329c601f7c0be024ec87832853c093d1427c4 Mon Sep 17 00:00:00 2001 From: Brad Harris Date: Fri, 4 Sep 2026 07:38:00 -0600 Subject: [PATCH 1/2] feat(chat): drop the pin picker from the composer MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Users no longer attach pins to chat messages — "I don't need to include already pinned things in a message." User attachments are files and links only. Agent-side pin attachments (dispatch_chat_post) are untouched and still render in the feed; the server's accepted schema is unchanged. - chat-composer: remove the pin button/popover, `pins` prop, pinIds state and the pin branch of the send payload. - chat-composer-attachments: remove attachablePins, PinChip, PinPickerButton. - chat-pane: stop passing pins to the composer (the feed keeps them). - chat-draft: drop `pinIds` from the persisted shape and the size accounting. Legacy drafts that still carry `pinIds` validate, are read without it (`readChatComposerDraft`) and never write it back. - tests: rewrite the pin cases; add legacy-draft coverage. - docs: chat-surface-plan states user attachments are files and links. Co-Authored-By: Claude Fable 5.1 --- .../chat/chat-composer-attachments.test.tsx | 47 +------ .../app/chat/chat-composer-attachments.tsx | 118 +----------------- .../app/chat/chat-composer-draft.test.tsx | 63 ++++++---- .../src/components/app/chat/chat-composer.tsx | 87 ++----------- .../web/src/components/app/chat/chat-pane.tsx | 1 - apps/web/src/lib/chat-draft.test.ts | 49 +++++--- apps/web/src/lib/chat-draft.ts | 37 ++++-- docs/chat-surface-plan.md | 17 ++- 8 files changed, 123 insertions(+), 296 deletions(-) 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..eeea1850 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} /> @@ -168,7 +157,6 @@ describe("ChatComposer draft persistence", () => { 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 +171,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" }); @@ -394,13 +412,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 +429,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 () => { diff --git a/apps/web/src/components/app/chat/chat-composer.tsx b/apps/web/src/components/app/chat/chat-composer.tsx index 64ba905f..57350cc6 100644 --- a/apps/web/src/components/app/chat/chat-composer.tsx +++ b/apps/web/src/components/app/chat/chat-composer.tsx @@ -6,7 +6,6 @@ import { useCallback, useEffect, useLayoutEffect, - useMemo, useRef, useState, } from "react"; @@ -25,8 +24,6 @@ import { import { DraftPlaceholderChip, PastedTextChip, - PinChip, - PinPickerButton, } from "@/components/app/chat/chat-composer-attachments"; import { ContextFileItem, @@ -37,14 +34,13 @@ import { 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,8 +85,6 @@ 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 { return { @@ -259,11 +251,11 @@ 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 + * The draft — text, links, 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 @@ -274,7 +266,6 @@ export function ChatComposer({ agentId, onSend, uploadFile, - pins = NO_PINS, disabledReason, sending = false, placeholder = "Message the agent…", @@ -288,20 +279,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) => ({ @@ -426,15 +413,7 @@ export function ChatComposer({ }); }, []); - const attachedPins = useMemo( - () => - pinIds - .map((id) => pins.find((pin) => pin.id === id)) - .filter((pin): pin is AgentPin => pin !== undefined), - [pinIds, pins] - ); - const attachmentCount = - files.length + placeholders.length + links.length + attachedPins.length; + const attachmentCount = files.length + placeholders.length + links.length; const attachmentsFull = attachmentCount >= CHAT_ATTACHMENTS_MAX; const noteAttachmentLimit = useCallback(() => { @@ -500,33 +479,6 @@ 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!) - ? 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); @@ -639,11 +591,6 @@ export function ChatComposer({ [addFiles, disabled] ); - const attachedPinIds = useMemo( - () => new Set(attachedPins.map((pin) => pin.id!)), - [attachedPins] - ); - const canSend = !disabled && !sending && @@ -672,7 +619,6 @@ export function ChatComposer({ const submittedText = text; const submittedFiles = files; const submittedLinks = links; - const submittedPins = attachedPins; const run = async () => { const attachments: ChatUserAttachmentInput[] = []; @@ -702,15 +648,11 @@ 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 @@ -720,7 +662,6 @@ export function ChatComposer({ ...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, })); for (const file of submittedFiles) removeFile(file); @@ -737,7 +678,6 @@ export function ChatComposer({ textareaRef.current?.focus(); }); }, [ - attachedPins, canSend, files, links, @@ -856,9 +796,6 @@ export function ChatComposer({ onRemove={() => removeLink(link)} /> ))} - {attachedPins.map((pin) => ( - removePin(pin)} /> - ))} ) : null}
@@ -885,12 +822,6 @@ export function ChatComposer({ > -