diff --git a/packages/core/chat/index.ts b/packages/core/chat/index.ts index 1f66e56250..09789fa2cd 100644 --- a/packages/core/chat/index.ts +++ b/packages/core/chat/index.ts @@ -1,4 +1,4 @@ -export { createChatStore, CHAT_MIN_W, CHAT_MIN_H, CHAT_DEFAULT_W, CHAT_DEFAULT_H, DRAFT_NEW_SESSION, newSessionDraftKey } from "./store"; +export { createChatStore, CHAT_MIN_W, CHAT_MIN_H, CHAT_DEFAULT_W, CHAT_DEFAULT_H, DRAFT_NEW_SESSION } from "./store"; export type { ChatStoreOptions, ChatState, ChatTimelineItem } from "./store"; export { useRecentContextStore, selectRecentContexts } from "./recent-context-store"; export type { RecentContextEntry, RecentContextType } from "./recent-context-store"; diff --git a/packages/core/chat/store.test.ts b/packages/core/chat/store.test.ts index 762977eaf1..3e11ef04e2 100644 --- a/packages/core/chat/store.test.ts +++ b/packages/core/chat/store.test.ts @@ -1,5 +1,5 @@ import { beforeEach, describe, expect, it } from "vitest"; -import { createChatStore, newSessionDraftKey } from "./store"; +import { createChatStore, DRAFT_NEW_SESSION } from "./store"; import type { StorageAdapter } from "../types"; import type { Attachment } from "../types"; @@ -36,10 +36,112 @@ function makeAttachment(id: string): Attachment { }; } -describe("newSessionDraftKey", () => { - it("derives a stable per-agent slot for an uncreated chat", () => { - expect(newSessionDraftKey("agent-1")).toBe("__new__:agent-1"); - expect(newSessionDraftKey(null)).toBe("__new__:"); +// The pre-MUL-4864 scheme kept one new-chat draft per agent, in `__new__:` +// slots. Those slots have no timestamp, so on upgrade only one can survive: +// the one for the agent the workspace has selected — the draft the user would +// have been shown. The rest were the invisible multi-draft state, and go. +describe("chat store — legacy per-agent new-chat draft migration", () => { + const DRAFTS_KEY = "multica:chat:drafts"; + const ATTACHMENTS_KEY = "multica:chat:draft-attachments"; + const AGENT_KEY = "multica:chat:selectedAgentId"; + + it("adopts the selected agent's legacy draft into the single new-chat slot", () => { + const storage = memStorage(); + storage.setItem(AGENT_KEY, "agent-1"); + storage.setItem( + DRAFTS_KEY, + JSON.stringify({ "__new__:agent-1": "mine", "__new__:agent-2": "other" }), + ); + + const store = createChatStore({ storage }); + + expect(store.getState().inputDrafts).toEqual({ [DRAFT_NEW_SESSION]: "mine" }); + }); + + it("migrates the matching attachments with the text, not another agent's", () => { + const storage = memStorage(); + storage.setItem(AGENT_KEY, "agent-1"); + storage.setItem(DRAFTS_KEY, JSON.stringify({ "__new__:agent-1": "mine" })); + storage.setItem( + ATTACHMENTS_KEY, + JSON.stringify({ + "__new__:agent-1": [makeAttachment("att-mine")], + "__new__:agent-2": [makeAttachment("att-other")], + }), + ); + + const store = createChatStore({ storage }); + + expect(store.getState().inputDraftAttachments[DRAFT_NEW_SESSION]?.map((a) => a.id)).toEqual([ + "att-mine", + ]); + expect(store.getState().inputDraftAttachments["__new__:agent-2"]).toBeUndefined(); + }); + + it("persists the migration so the legacy slots do not come back on reload", () => { + const storage = memStorage(); + storage.setItem(AGENT_KEY, "agent-1"); + storage.setItem( + DRAFTS_KEY, + JSON.stringify({ "__new__:agent-1": "mine", "__new__:agent-2": "other" }), + ); + + createChatStore({ storage }); + // A second store reads what the first one wrote — this is the reload. + const reloaded = createChatStore({ storage }); + + expect(JSON.parse(storage.getItem(DRAFTS_KEY) ?? "{}")).toEqual({ [DRAFT_NEW_SESSION]: "mine" }); + expect(reloaded.getState().inputDrafts).toEqual({ [DRAFT_NEW_SESSION]: "mine" }); + }); + + it("drops every legacy slot when no agent is selected", () => { + const storage = memStorage(); + storage.setItem(DRAFTS_KEY, JSON.stringify({ "__new__:agent-1": "a", "__new__:agent-2": "b" })); + + const store = createChatStore({ storage }); + + expect(store.getState().inputDrafts).toEqual({}); + expect(storage.getItem(DRAFTS_KEY)).toBeNull(); + }); + + it("leaves real session drafts untouched", () => { + const storage = memStorage(); + storage.setItem(AGENT_KEY, "agent-1"); + storage.setItem( + DRAFTS_KEY, + JSON.stringify({ "session-a": "draft A", "session-b": "draft B", "__new__:agent-1": "mine" }), + ); + + const store = createChatStore({ storage }); + + expect(store.getState().inputDrafts).toEqual({ + "session-a": "draft A", + "session-b": "draft B", + [DRAFT_NEW_SESSION]: "mine", + }); + }); + + it("keeps a current-scheme draft rather than overwriting it with a legacy one", () => { + const storage = memStorage(); + storage.setItem(AGENT_KEY, "agent-1"); + storage.setItem( + DRAFTS_KEY, + JSON.stringify({ [DRAFT_NEW_SESSION]: "current", "__new__:agent-1": "stale" }), + ); + + const store = createChatStore({ storage }); + + expect(store.getState().inputDrafts).toEqual({ [DRAFT_NEW_SESSION]: "current" }); + }); + + it("does not touch storage when there is nothing to migrate", () => { + const storage = memStorage(); + storage.setItem(DRAFTS_KEY, JSON.stringify({ [DRAFT_NEW_SESSION]: "typed" })); + const before = storage.getItem(DRAFTS_KEY); + + createChatStore({ storage }); + + expect(storage.getItem(DRAFTS_KEY)).toBe(before); }); }); diff --git a/packages/core/chat/store.ts b/packages/core/chat/store.ts index 127c197af2..5bdea3dfb6 100644 --- a/packages/core/chat/store.ts +++ b/packages/core/chat/store.ts @@ -35,18 +35,17 @@ const APPLIED_RESTORES_KEY = "multica:chat:applied-draft-restores"; * they are refetchable, so dropping one loses nothing. */ const PENDING_SEND_RESTORES_KEY = "multica:chat:pending-send-restores"; -/** Placeholder sessionId for a chat that hasn't been created yet. */ +/** + * Draft slot for a chat that hasn't been created yet. There is exactly one per + * workspace: the new-chat composer's identity is "the chat I have not created", + * not "the chat I have not created with agent X". `selectedAgentId` is the send + * target, not draft ownership, so switching agent mid-compose keeps the text + * (MUL-4864). Created sessions keep their own slot, keyed by session id. + */ export const DRAFT_NEW_SESSION = "__new__"; -/** - * Draft storage key for an as-yet-uncreated chat with the given agent. - * Shared by ChatInput (which writes the draft) and ensureSession (which - * migrates it onto the real session id the moment the session is created), - * so the two never disagree on the slot name. - */ -export function newSessionDraftKey(selectedAgentId: string | null): string { - return `${DRAFT_NEW_SESSION}:${selectedAgentId ?? ""}`; -} +/** Pre-MUL-4864 per-agent new-chat slots, shaped `__new__:`. */ +const LEGACY_NEW_SESSION_PREFIX = `${DRAFT_NEW_SESSION}:`; const CHAT_WIDTH_KEY = "multica:chat:width"; const CHAT_HEIGHT_KEY = "multica:chat:height"; const CHAT_EXPANDED_KEY = "multica:chat:expanded"; @@ -197,6 +196,64 @@ function writeDraftAttachments( } } +/** + * Fold the legacy per-agent new-chat slots into the single DRAFT_NEW_SESSION + * slot, then drop them. + * + * The legacy slots carry no timestamp, so when several exist there is no way to + * tell which one the user typed last — and "keep them all" has nowhere to put + * the losers now that there is one composer. Adopt the slot belonging to the + * persisted `selectedAgentId` (the draft this workspace would have shown on + * open, so the only one the user can be expecting) and discard the rest: those + * extra slots ARE the invisible multi-draft state this change removes. + * + * Both write paths prune empty values, so key presence means content. + * Idempotent: once the legacy keys are gone this is an allocation-free no-op. + */ +function migrateLegacyNewChatSlots( + slots: Record, + selectedAgentId: string | null, +): { slots: Record; changed: boolean } { + const legacyKeys = Object.keys(slots).filter((k) => k.startsWith(LEGACY_NEW_SESSION_PREFIX)); + if (legacyKeys.length === 0) return { slots, changed: false }; + const next = { ...slots }; + const adopted = next[`${LEGACY_NEW_SESSION_PREFIX}${selectedAgentId ?? ""}`]; + // Never clobber the unified slot: whatever is in it was written under the + // current scheme and is therefore newer than any legacy leftover. + if (!(DRAFT_NEW_SESSION in next) && adopted !== undefined) { + next[DRAFT_NEW_SESSION] = adopted; + } + for (const key of legacyKeys) delete next[key]; + logger.info("migrating legacy per-agent new-chat drafts", { + legacyCount: legacyKeys.length, + selectedAgentId, + adopted: DRAFT_NEW_SESSION in next, + }); + return { slots: next, changed: true }; +} + +/** + * Read both draft maps and migrate them together, against the same + * `selectedAgentId` — text and attachments must never disagree on which legacy + * new-chat draft survived, or the user gets agent A's words with agent B's + * files. + */ +function loadDraftSlots( + storage: StorageAdapter, + draftsKey: string, + attachmentsKey: string, + selectedAgentId: string | null, +): { inputDrafts: Record; inputDraftAttachments: Record } { + const drafts = migrateLegacyNewChatSlots(readDrafts(storage, draftsKey), selectedAgentId); + const attachments = migrateLegacyNewChatSlots( + readDraftAttachments(storage, attachmentsKey), + selectedAgentId, + ); + if (drafts.changed) writeDrafts(storage, draftsKey, drafts.slots); + if (attachments.changed) writeDraftAttachments(storage, attachmentsKey, attachments.slots); + return { inputDrafts: drafts.slots, inputDraftAttachments: attachments.slots }; +} + export const CHAT_MIN_W = 360; export const CHAT_MIN_H = 480; export const CHAT_DEFAULT_W = 380; @@ -294,13 +351,21 @@ export function createChatStore(options: ChatStoreOptions) { // (new user) resolves to enabled. const initialFloatingEnabled = storage.getItem(FLOATING_KEY) !== "false"; + const initialAgentId = storage.getItem(wsKey(AGENT_STORAGE_KEY)); + const initialDraftSlots = loadDraftSlots( + storage, + wsKey(DRAFTS_KEY), + wsKey(DRAFT_ATTACHMENTS_KEY), + initialAgentId, + ); + const store = create((set, get) => ({ isOpen: initialIsOpen, floatingChatEnabled: initialFloatingEnabled, activeSessionId: storage.getItem(wsKey(SESSION_STORAGE_KEY)), - selectedAgentId: storage.getItem(wsKey(AGENT_STORAGE_KEY)), - inputDrafts: readDrafts(storage, wsKey(DRAFTS_KEY)), - inputDraftAttachments: readDraftAttachments(storage, wsKey(DRAFT_ATTACHMENTS_KEY)), + selectedAgentId: initialAgentId, + inputDrafts: initialDraftSlots.inputDrafts, + inputDraftAttachments: initialDraftSlots.inputDraftAttachments, appliedDraftRestoreIds: readAppliedRestores(storage, wsKey(APPLIED_RESTORES_KEY)), pendingSendRestores: readPendingSendRestores(storage, wsKey(PENDING_SEND_RESTORES_KEY)), chatWidth: Number(storage.getItem(CHAT_WIDTH_KEY)) || CHAT_DEFAULT_W, @@ -456,8 +521,15 @@ export function createChatStore(options: ChatStoreOptions) { registerForWorkspaceRehydration(() => { const nextSession = storage.getItem(wsKey(SESSION_STORAGE_KEY)); const nextAgent = storage.getItem(wsKey(AGENT_STORAGE_KEY)); - const nextDrafts = readDrafts(storage, wsKey(DRAFTS_KEY)); - const nextDraftAttachments = readDraftAttachments(storage, wsKey(DRAFT_ATTACHMENTS_KEY)); + // Drafts are namespaced per workspace, so the workspace being switched TO + // has its own legacy slots to fold — migrate against that workspace's own + // persisted agent, not the one we are leaving. + const { inputDrafts: nextDrafts, inputDraftAttachments: nextDraftAttachments } = loadDraftSlots( + storage, + wsKey(DRAFTS_KEY), + wsKey(DRAFT_ATTACHMENTS_KEY), + nextAgent, + ); logger.info("workspace rehydration", { prevSession: store.getState().activeSessionId, nextSession, diff --git a/packages/views/chat/components/chat-input.test.tsx b/packages/views/chat/components/chat-input.test.tsx index 721f6161f9..d16d8bd5a4 100644 --- a/packages/views/chat/components/chat-input.test.tsx +++ b/packages/views/chat/components/chat-input.test.tsx @@ -143,7 +143,6 @@ vi.mock("@multica/core/chat", () => { }; return { DRAFT_NEW_SESSION: "__draft_new__", - newSessionDraftKey: (agentId: string | null) => `__draft_new__:${agentId ?? ""}`, useChatStore: Object.assign( (selector?: (s: typeof state) => unknown) => selector ? selector(state) : state, @@ -224,6 +223,93 @@ function element(props: Partial>) { ); } +// MUL-4864: an uncreated chat has ONE draft per workspace. `selectedAgentId` +// picks where the first send goes; it does not own the draft. Switching agent +// mid-compose must therefore change nothing the user can see. +describe("ChatInput new-chat draft identity", () => { + function switchAgentTo(agentId: string, rerender: (ui: React.ReactElement) => void) { + const state = useChatStore.getState() as unknown as { selectedAgentId: string }; + state.selectedAgentId = agentId; + // The mock store is not reactive; a real store switch re-renders the tree. + rerender(element({ agentName: agentId })); + } + + it("writes to the single new-chat slot regardless of the selected agent", () => { + const { rerender } = render(element({ agentName: "agent-1" })); + + fireEvent.change(screen.getByTestId("editor"), { target: { value: "half a thought" } }); + switchAgentTo("agent-2", rerender); + fireEvent.change(screen.getByTestId("editor"), { target: { value: "half a thought, finished" } }); + + const state = useChatStore.getState() as unknown as { inputDrafts: Record }; + // One slot, not one per agent — the hidden multi-draft state is gone. + expect(Object.keys(state.inputDrafts)).toEqual(["__draft_new__"]); + expect(state.inputDrafts["__draft_new__"]).toBe("half a thought, finished"); + }); + + it("keeps the live editor instance across an agent switch", () => { + const { rerender } = render(element({ agentName: "agent-1" })); + const before = screen.getByTestId("editor"); + + switchAgentTo("agent-2", rerender); + + // Identity, not just content: a remount would silently drop whatever the + // 100ms draft debounce had not yet persisted — the last thing typed. + expect(screen.getByTestId("editor")).toBe(before); + }); + + it("keeps text the draft debounce has not persisted yet across an agent switch", () => { + const { rerender } = render(element({ agentName: "agent-1" })); + // The uncontrolled textarea models the live editor document: text lives in + // the instance, and only a remount can lose it. + const editor = screen.getByTestId("editor") as HTMLTextAreaElement; + fireEvent.change(editor, { target: { value: "unsaved words" } }); + + switchAgentTo("agent-2", rerender); + + expect((screen.getByTestId("editor") as HTMLTextAreaElement).value).toBe("unsaved words"); + }); + + it("keeps staged attachments across an agent switch", async () => { + const onUploadFile = vi.fn(async (_file: File) => + makeUpload({ id: "att-kept", link: "/api/attachments/att-kept/download", filename: "a.png" }), + ); + const { rerender } = render(element({ agentName: "agent-1", onUploadFile })); + + await act(async () => { + dropHandlers.onDrop?.([new File(["x"], "a.png", { type: "image/png" })]); + await Promise.resolve(); + }); + switchAgentTo("agent-2", rerender); + + const state = useChatStore.getState() as unknown as { + inputDraftAttachments: Record; + }; + // Body and attachments share one attribution rule, so the files follow the + // text across the switch instead of stranding in the old agent's slot. + expect(state.inputDraftAttachments["__draft_new__"]?.map((a) => a.id)).toEqual(["att-kept"]); + expect(Object.keys(state.inputDraftAttachments)).toEqual(["__draft_new__"]); + }); + + it("still gives each created session its own draft slot", () => { + const state = useChatStore.getState() as unknown as { + activeSessionId: string | null; + inputDrafts: Record; + }; + state.activeSessionId = "session-a"; + const { rerender } = render(element({ agentName: "agent-1" })); + fireEvent.change(screen.getByTestId("editor"), { target: { value: "for A" } }); + + state.activeSessionId = "session-b"; + rerender(element({ agentName: "agent-1" })); + fireEvent.change(screen.getByTestId("editor"), { target: { value: "for B" } }); + + // Real sessions stay isolated — unifying the NEW-chat draft must not bleed + // one conversation's context into another. + expect(state.inputDrafts).toEqual({ "session-a": "for A", "session-b": "for B" }); + }); +}); + describe("ChatInput focusRequest", () => { it("focuses the editor when focusRequest becomes a non-zero value (new chat)", () => { const { rerender } = render( @@ -317,7 +403,7 @@ describe("ChatInput attachment wiring", () => { const [, ids] = onSend.mock.calls[0]!; expect(ids).toEqual(["att-42"]); expect(useChatStore.getState().addInputDraftAttachment).toHaveBeenCalledWith( - "__draft_new__:agent-1", + "__draft_new__", expect.objectContaining({ id: "att-42" }), ); }); @@ -443,7 +529,7 @@ describe("ChatInput async send", () => { await waitFor(() => { expect(useChatStore.getState().setInputDraft).toHaveBeenCalledWith( - "__draft_new__:agent-1", + "__draft_new__", "bring this back", ); expect(editorProps.last?.defaultValue).toBe("bring this back"); @@ -461,7 +547,7 @@ describe("ChatInput async send", () => { inputDrafts: Record; setInputDraft: ReturnType; }; - state.inputDrafts["__draft_new__:agent-1"] = "already typing"; + state.inputDrafts["__draft_new__"] = "already typing"; const onRestoreDraftApplied = vi.fn(); const { rerender } = render( @@ -476,13 +562,13 @@ describe("ChatInput async send", () => { }); expect(onRestoreDraftApplied).not.toHaveBeenCalled(); expect(state.setInputDraft).not.toHaveBeenCalledWith( - "__draft_new__:agent-1", + "__draft_new__", "bring this back", ); // The user sends/clears what they were typing: the same restore, still // pending, now lands. - state.inputDrafts["__draft_new__:agent-1"] = ""; + state.inputDrafts["__draft_new__"] = ""; rerender( element({ restoreDraftRequest: { id: "msg-restored", content: "bring this back" }, @@ -492,7 +578,7 @@ describe("ChatInput async send", () => { await waitFor(() => { expect(state.setInputDraft).toHaveBeenCalledWith( - "__draft_new__:agent-1", + "__draft_new__", "bring this back", ); expect(onRestoreDraftApplied).toHaveBeenCalledTimes(1); @@ -505,7 +591,7 @@ describe("ChatInput async send", () => { setInputDraft: ReturnType; setInputDraftAttachments: ReturnType; }; - state.inputDraftAttachments["__draft_new__:agent-1"] = [{ id: "att-staged" }]; + state.inputDraftAttachments["__draft_new__"] = [{ id: "att-staged" }]; const onRestoreDraftApplied = vi.fn(); renderInput({ @@ -523,7 +609,7 @@ describe("ChatInput async send", () => { // The staged attachment list must never be replaced by the restore. expect(state.setInputDraftAttachments).not.toHaveBeenCalled(); expect(state.setInputDraft).not.toHaveBeenCalledWith( - "__draft_new__:agent-1", + "__draft_new__", "bring this back", ); }); @@ -561,7 +647,7 @@ describe("ChatInput async send", () => { commitInput({ extraDraftKeys: ["session-1"] }); }); - expect(useChatStore.getState().clearInputDraft).toHaveBeenCalledWith("__draft_new__:agent-1"); + expect(useChatStore.getState().clearInputDraft).toHaveBeenCalledWith("__draft_new__"); expect(useChatStore.getState().clearInputDraft).toHaveBeenCalledWith("session-1"); await act(async () => { @@ -604,8 +690,8 @@ describe("ChatInput async send", () => { link: "/api/attachments/att-persisted/download", filename: "persisted.png", }); - state.inputDrafts["__draft_new__:agent-1"] = "see ![](/api/attachments/att-persisted/download)"; - state.inputDraftAttachments["__draft_new__:agent-1"] = [attachment]; + state.inputDrafts["__draft_new__"] = "see ![](/api/attachments/att-persisted/download)"; + state.inputDraftAttachments["__draft_new__"] = [attachment]; const onSend = vi.fn((_content, _ids, commitInput) => { commitInput(); @@ -781,7 +867,7 @@ describe("ChatInput commit handoff", () => { expect(editorState.cleared).toBeGreaterThan(0); expect(editorState.blurred).toBeGreaterThan(0); - expect(useChatStore.getState().clearInputDraft).toHaveBeenCalledWith("__draft_new__:agent-1"); + expect(useChatStore.getState().clearInputDraft).toHaveBeenCalledWith("__draft_new__"); }); it("leaves the editor intact on a fire-and-forget commit but still clears the sent draft", async () => { @@ -795,6 +881,6 @@ describe("ChatInput commit handoff", () => { expect(editorState.cleared).toBe(0); expect(editorState.blurred).toBe(0); // …but the sent session's persisted draft is cleared regardless. - expect(useChatStore.getState().clearInputDraft).toHaveBeenCalledWith("__draft_new__:agent-1"); + expect(useChatStore.getState().clearInputDraft).toHaveBeenCalledWith("__draft_new__"); }); }); diff --git a/packages/views/chat/components/chat-input.tsx b/packages/views/chat/components/chat-input.tsx index 0914c31727..88b897ad1f 100644 --- a/packages/views/chat/components/chat-input.tsx +++ b/packages/views/chat/components/chat-input.tsx @@ -12,7 +12,7 @@ import { } from "../../editor"; import { SubmitButton } from "@multica/ui/components/common/submit-button"; import { ChatAddMenu } from "./chat-add-menu"; -import { useChatStore, newSessionDraftKey } from "@multica/core/chat"; +import { useChatStore, DRAFT_NEW_SESSION } from "@multica/core/chat"; import { createLogger } from "@multica/core/logger"; import { formatShortcut, useShortcut } from "@multica/core/shortcuts"; import type { UploadResult } from "@multica/core/hooks/use-file-upload"; @@ -22,6 +22,8 @@ import { useT } from "../../i18n"; const logger = createLogger("chat.ui"); const EMPTY_ATTACHMENTS: Attachment[] = []; +/** Editor identity for the chat composer — see the editorKey note below. */ +const CHAT_COMPOSER_EDITOR_KEY = "chat-composer"; function attachmentReferenceUrls(attachment: Attachment): string[] { const withUploadFields = attachment as Attachment & { @@ -129,36 +131,35 @@ export function ChatInput({ const sendShortcut = useShortcut("send"); const editorRef = useRef(null); const activeSessionId = useChatStore((s) => s.activeSessionId); - const selectedAgentId = useChatStore((s) => s.selectedAgentId); // Two keys with deliberately different concerns: // - // `draftKey` — zustand storage key. Scopes the in-progress draft per - // session so different sessions don't bleed text into each other; for - // brand-new chats it falls back to a per-agent slot so switching agents - // mid-compose gives each agent its own draft. This is a STORAGE key, not - // a React identity. + // `draftKey` — zustand storage key. Scopes the in-progress draft per session + // so different sessions don't bleed text into each other. An uncreated chat + // uses ONE slot per workspace, deliberately NOT keyed by agent: the composer + // is "the chat I have not created yet", and `selectedAgentId` only decides + // where the first send goes (MUL-4864). This is a STORAGE key, not a React + // identity. // - // `editorKey` — React `key` on the ContentEditor. Forces a fresh editor - // instance when the user explicitly switches agent. Placeholder text itself - // no longer depends on this: ContentEditor's placeholder-sync effect - // refreshes it live (e.g. across archived ↔ active sessions of the SAME - // agent, where this key does not change). A cancelled-run draft restore - // does NOT bump this key either: it just writes - // the restored text into `inputDraft`, and the editor's own - // defaultValue-sync effect (content-editor.tsx) pushes it into the live - // instance. There is no second copy of the draft to drift or resurface. - // Crucially this does NOT include `activeSessionId`: when the user - // uploads a file in a brand-new chat, `handleUploadFile` first awaits - // `ensureSession` which lazily creates the session and flips - // `activeSessionId` from null → uuid mid-upload. If the editor key - // depended on session id, that flip would unmount the editor right as - // the blob preview was inserted, dropping the in-progress upload's - // image node before file-upload.ts could swap it for the CDN URL — the - // user would see the image flash on then disappear. Keeping editor - // identity stable across the lazy-create event is what makes - // first-upload-creates-session work the same as second-upload. - const draftKey = - draftKeyOverride ?? activeSessionId ?? newSessionDraftKey(selectedAgentId); + // `editorKey` — React `key` on the ContentEditor, i.e. editor identity. It is + // constant for the chat composer, because nothing about switching what you + // are composing to should throw away the instance you are typing in: + // - Agent switch: same draft slot now, so a remount would only serve to + // drop the last <100ms of typing the draft debounce has not persisted. + // - Placeholder: ContentEditor's placeholder-sync effect refreshes it live, + // so it never needed a remount. + // - Draft restore (a cancelled run, a failed send): writes into + // `inputDraft`, and the editor's defaultValue-sync effect pushes it into + // the live instance. There is no second copy to drift or resurface. + // - Session switch / lazy create: when the user uploads a file in a + // brand-new chat, `handleUploadFile` awaits `ensureSession`, which flips + // `activeSessionId` from null → uuid mid-upload. A session-keyed editor + // would unmount right as the blob preview landed, dropping the image node + // before file-upload.ts could swap in the CDN URL — the user would watch + // the image flash on and vanish. Stable identity is what makes + // first-upload-creates-session behave like every later upload. + // Embedded surfaces (Agent Builder) still pass `editorKeyOverride` to isolate + // their own composer. + const draftKey = draftKeyOverride ?? activeSessionId ?? DRAFT_NEW_SESSION; // Select a primitive — empty-string fallback keeps referential stability. const inputDraft = useChatStore((s) => s.inputDrafts[draftKey] ?? ""); const draftAttachments = useChatStore( @@ -181,7 +182,7 @@ export function ChatInput({ // reads the live editor and bails when it is empty. const hasNothingToSend = isEmpty && !inputDraft.trim(); const appliedRestoreIdRef = useRef(null); - const editorKey = editorKeyOverride ?? selectedAgentId ?? "no-agent"; + const editorKey = editorKeyOverride ?? CHAT_COMPOSER_EDITOR_KEY; // Submit gate. `uploading` disables the SubmitButton the instant an upload // starts; `isBlocked()` is re-read inside handleSend for the paths that skip // the button entirely (Mod+Enter mid-paste, drag-drop racing the keyboard). @@ -409,8 +410,8 @@ export function ChatInput({ >
{ expect(hasOptimisticInFlight(qc, sid)).toBe(true); }); }); + +// The post-send "scrub the composer?" rule, shared by BOTH send chains (the +// chat tab's controller and the floating ChatWindow) so they cannot drift. +// MUL-4864: the new-chat composer is one box per workspace, so the selected +// agent is NOT part of compose-target identity — only the session is. +describe("isStillOnComposeTarget", () => { + it("is true when the user never left the session they sent from", () => { + expect(isStillOnComposeTarget(sid, sid)).toBe(true); + }); + + it("is true for a new chat the user is still sitting in", () => { + // Both null: ensureSession creates the row but does not publish it as + // active, so a user who stayed put is still looking at the new-chat box. + expect(isStillOnComposeTarget(null, null)).toBe(true); + }); + + it("is false once the user opens a different session mid-send", () => { + expect(isStillOnComposeTarget("session-2", sid)).toBe(false); + }); + + it("is false when the user starts a new chat mid-send from a session", () => { + expect(isStillOnComposeTarget(null, sid)).toBe(false); + }); +}); diff --git a/packages/views/chat/components/use-chat-controller.test.tsx b/packages/views/chat/components/use-chat-controller.test.tsx index e6daf6a846..b471e62fcd 100644 --- a/packages/views/chat/components/use-chat-controller.test.tsx +++ b/packages/views/chat/components/use-chat-controller.test.tsx @@ -53,6 +53,9 @@ const h = vi.hoisted(() => { store, archivedMutate: vi.fn(), markReadMutate: vi.fn(), + // Stable across renders so tests can assert on it; lazy-creates the session + // a new chat's first send needs. + createSessionMutate: vi.fn(async () => ({ id: "new-session" })), // Foreground gate for the auto mark-read effect; tests flip it. appForeground: { value: true }, consumeRestoreMutate: vi.fn(), @@ -78,6 +81,9 @@ vi.mock("@multica/core/workspace/queries", () => ({ vi.mock("@multica/views/issues/components", () => ({ canAssignAgent: () => true })); vi.mock("@multica/core/api", () => ({ api: { sendChatMessage: vi.fn(), cancelTaskById: vi.fn() }, + // Names the 403 that a revoked invoke permission raises (MUL-4525); plain + // failures have no reason code. + dispatchReasonCode: () => undefined, })); vi.mock("@multica/core/agents", () => ({ useAgentPresenceDetail: () => ({ availability: "online" }), @@ -87,7 +93,7 @@ vi.mock("@multica/core/hooks/use-file-upload", () => ({ useFileUpload: () => ({ uploadWithToast: vi.fn() }), })); vi.mock("@multica/core/chat/mutations", () => ({ - useCreateChatSession: () => ({ mutateAsync: vi.fn() }), + useCreateChatSession: () => ({ mutateAsync: h.createSessionMutate }), useMarkChatSessionRead: () => ({ mutate: h.markReadMutate }), useSetChatSessionArchived: () => ({ mutate: h.archivedMutate }), useConsumeChatDraftRestore: () => ({ mutate: h.consumeRestoreMutate }), @@ -140,6 +146,7 @@ vi.mock("@tanstack/react-query", async (importOriginal) => { }); import { useChatController } from "./use-chat-controller"; +import { api } from "@multica/core/api"; // --- Fixtures --------------------------------------------------------------- function makeSession( @@ -466,3 +473,105 @@ describe("useChatController durable draft restores (#5219)", () => { expect(result.current.restoreDraftRequest?.serverRestoreId).toBe("msg-1"); }); }); + +// After a send, the composer is scrubbed only if the user is still on the +// session they sent from — otherwise the shared editor is showing a different +// draft and clearing it would wipe visible input. MUL-4864 changes what +// "still here" means: with ONE new-chat draft, the composer no longer belongs +// to an agent, so only `activeSessionId` can answer it. +describe("useChatController.handleSend — compose target tracking", () => { + beforeEach(() => { + h.store.setActiveSession.mockClear(); + h.createSessionMutate.mockClear(); + h.createSessionMutate.mockResolvedValue({ id: "new-session" }); + vi.mocked(api.sendChatMessage).mockResolvedValue({ + message_id: "msg-1", + task_id: "task-1", + created_at: new Date(0).toISOString(), + } as unknown as Awaited>); + }); + + function sendFrom(activeSessionId: string | null, whileSending?: () => void) { + h.store.activeSessionId = activeSessionId; + h.store.selectedAgentId = "agent-a"; + h.sessions = [sA]; + h.agents = [agentA]; + const { result } = renderHook(() => useChatController()); + h.store.setActiveSession.mockClear(); + const commitInput = vi.fn(); + return { + commitInput, + send: () => + act(async () => { + const pending = result.current.handleSend("hello", undefined, commitInput); + // Runs between ensureSession and the commit — the window the user + // actually races with when they touch the picker after hitting send. + whileSending?.(); + await pending; + }), + }; + } + + it("scrubs the composer after a new chat's first send", async () => { + const { commitInput, send } = sendFrom(null); + await send(); + + expect(commitInput).toHaveBeenCalledWith( + expect.objectContaining({ clearEditor: true, extraDraftKeys: ["new-session"] }), + ); + expect(h.store.setActiveSession).toHaveBeenCalledWith("new-session"); + }); + + it("scrubs the composer even if the agent picker moved mid-send", async () => { + // The sent text is still sitting in the one shared composer. Leaving it + // there would arm a second, unintended send to the agent just picked. + const { commitInput, send } = sendFrom(null, () => { + h.store.selectedAgentId = "agent-b"; + }); + await send(); + + expect(commitInput).toHaveBeenCalledWith( + expect.objectContaining({ clearEditor: true, extraDraftKeys: ["new-session"] }), + ); + // The message goes to the agent selected when Send was pressed — a later + // flick of the picker does not re-route work already on its way. + expect(h.createSessionMutate).toHaveBeenCalledWith( + expect.objectContaining({ agent_id: "agent-a" }), + ); + // …and that session is what opens, so the user sees the reply they asked for. + expect(h.store.setActiveSession).toHaveBeenCalledWith("new-session"); + }); + + it("keeps the input when session create fails, and opens nothing", async () => { + h.createSessionMutate.mockRejectedValue(new Error("create failed")); + const { commitInput, send } = sendFrom(null); + await send(); + + // No commit = the draft is never cleared, so the user's words survive. + expect(commitInput).not.toHaveBeenCalled(); + expect(h.store.setActiveSession).not.toHaveBeenCalled(); + }); + + it("leaves the composer alone when the user navigated to another session mid-send", async () => { + // Genuine navigation: the editor now shows sB's draft, which this send has + // no business clearing. Fire-and-forget — the reply surfaces as unread. + const { commitInput, send } = sendFrom(null, () => { + h.store.activeSessionId = "sB"; + }); + await send(); + + expect(commitInput).toHaveBeenCalledWith( + expect.objectContaining({ clearEditor: false, extraDraftKeys: ["new-session"] }), + ); + expect(h.store.setActiveSession).not.toHaveBeenCalled(); + }); + + it("still clears the sent draft when sending from an existing session", async () => { + const { commitInput, send } = sendFrom("sA"); + await send(); + + expect(commitInput).toHaveBeenCalledWith( + expect.objectContaining({ clearEditor: true, extraDraftKeys: ["sA"] }), + ); + }); +}); diff --git a/packages/views/chat/components/use-chat-controller.ts b/packages/views/chat/components/use-chat-controller.ts index fdd5ec279e..76520ce4ed 100644 --- a/packages/views/chat/components/use-chat-controller.ts +++ b/packages/views/chat/components/use-chat-controller.ts @@ -64,6 +64,28 @@ export function deriveChatTitle(content: string): string { return cleaned.slice(0, CHAT_TITLE_MAX - 1).trimEnd() + "…"; } +/** + * After a send resolves: is the user still composing to the target they sent + * from? Decides whether to scrub the composer and open the sent session, or + * treat the send as fire-and-forget (the reply surfaces as unread instead). + * + * The active session answers this on its own, deliberately. The new-chat + * composer is ONE box per workspace (see DRAFT_NEW_SESSION), so moving the + * agent picker re-points where the next send goes without moving the view or + * the draft slot — that is not "navigating away" (MUL-4864). Counting it as + * such would leave a completed send's text sitting in the composer, primed to + * be sent a second time to the agent just picked. + * + * Shared by both send chains — the chat tab's controller and the floating + * ChatWindow — so the rule cannot drift between the two surfaces. + */ +export function isStillOnComposeTarget( + liveActiveSessionId: string | null, + sentFromSessionId: string | null, +): boolean { + return liveActiveSessionId === sentFromSessionId; +} + // True when a session has an in-flight optimistic write — an `optimistic-` // message or a pending task in the cache. That is the signal of a just-created // (or actively-sending) session still awaiting server confirmation, before the @@ -497,10 +519,10 @@ export function useChatController(opts?: { isActive?: boolean }) { status: "queued", created_at: sentAt, }); + // Cache primed → safe to publish the new active session, but only if the + // user hasn't navigated away mid-send. See isStillOnComposeTarget. const live = useChatStore.getState(); - const stillOnSourceSession = - live.activeSessionId === activeSessionId && - (activeSessionId !== null || live.selectedAgentId === selectedAgentId); + const stillOnSourceSession = isStillOnComposeTarget(live.activeSessionId, activeSessionId); if (stillOnSourceSession) { setActiveSession(sessionId); } @@ -569,7 +591,6 @@ export function useChatController(opts?: { isActive?: boolean }) { }, [ activeSessionId, - selectedAgentId, activeAgent, isAgentArchived, ensureSession,