fix(webui): composer clears on send with draft recovery (webui-parity slice 13)

packages/webui/webapp/components/composer.tsx

@@ -14,12 +14,18 @@ import {
 import { createPortal } from "react-dom";
 
 import * as api from "@/lib/api";
+import { clientId } from "@/lib/cid";
 import {
   getComposerDraft,
   setComposerDraft,
   subscribeComposerDraft,
 } from "@/lib/composer-draft";
-import { useSessionContext } from "@/lib/store";
+import {
+  completeComposerSent,
+  failComposerSent,
+  startComposerSent,
+} from "@/lib/composer-sent";
+import { getActiveSessionId, useSessionContext } from "@/lib/store";
 import { decodeTranscript } from "@/lib/transcript";
 import { translate, type Locale, type MessageKey } from "@/lib/i18n";
 import { ContextMeter } from "./context-meter";
@@ -146,6 +152,19 @@ export function Composer({
   const [slashIndex, setSlashIndex] = useState(0);
   const editorRef = useRef<HTMLTextAreaElement>(null);
   const fileRef = useRef<HTMLInputElement>(null);
+  // The live session id used to live on a per-instance ref here.
+  // That was correct for chat→chat switching (the instance survives)
+  // but wrong for the home↔chat boundary: page.tsx swaps the
+  // composer between two tree positions when `hasConversation`
+  // flips, and creating a fresh session clears `chat`, which can
+  // unmount the composer mid-flight. The in-flight closure keeps a
+  // ref frozen at the dispatch-time session id and never sees the
+  // rotation. Reading from `getActiveSessionId()` at catch time
+  // resolves the live session id from the module-scope snapshot
+  // the SSE handler writes — the same store `composer-draft.ts`
+  // and `composer-sent.ts` already use to survive remounts.
+  // See lib/store.tsx#getActiveSessionId and the tripwire test
+  // `composer-submit-tripwire.test.ts` for the wiring pin.
   // Drag-and-drop overlay state. The counter lives in a ref so that the
   // dragenter/dragleave sequence can update it without scheduling a
   // re-render on every event — only the visible overlay (driven by
@@ -316,20 +335,101 @@ export function Composer({
   const submit = useCallback(async () => {
     const content = value.trim();
     if ((!content && attachments.length === 0) || readOnly || sending) return;
+    // Capture the dispatch context — what session this send was FOR.
+    // The outbox record stores these, so a later failure can identify
+    // its owner. They are NOT the values the catch branch compares
+    // against; the catch branch reads the LIVE context (see below).
+    const dispatchCid = clientId();
+    const dispatchSessionId = state?.sessionId ?? null;
     setSending(true);
     setComposerDraft({ error: null });
+    // Ticket 13 — optimistic clear. The backend does session
+    // switching and transcript backfill before its ack, so waiting
+    // for the await leaves the text sitting in the box for the whole
+    // in-flight window. Park the message in the outbox (a sibling
+    // module-scope store to `composer-draft.ts`, see `lib/composer-
+    // sent.ts`) and clear the composer immediately. On success the
+    // outbox flips to `delivered` and the SSE stream renders the user
+    // bubble; on failure the catch branch reads the stashed text back
+    // into the composer — see the design note at the top of
+    // `lib/composer-sent.ts`.
+    startComposerSent({
+      cid: dispatchCid,
+      sessionId: dispatchSessionId,
+      content,
+      attachments,
+    });
+    setComposerDraft({ value: "", attachments: [] });
     try {
       // A leading slash is a command, not a message: mcode parses those, and the
-      // webui's own slash commands are handled server-side too.
+      // webui's own slash commands are handled server-side too. The same
+      // record/clear/restore semantics apply to both branches.
       if (content.startsWith("/")) await api.sendCommand(content);
       else await api.sendMessage({ content, attachments });
-      setComposerDraft({ value: "", attachments: [] });
+      completeComposerSent();
     } catch (cause) {
-      setComposerDraft({ error: cause instanceof Error ? cause.message : String(cause) });
+      const errorMessage = cause instanceof Error ? cause.message : String(cause);
+      // Read the LIVE context at catch time. The dispatch-side
+      // closure has the session id from when the user pressed
+      // Enter; if the user has since switched sessions (e.g. via the
+      // sidebar), the active session id is now different and the
+      // failure belongs to the old session, not the one currently
+      // rendered. Comparing against the captured `dispatchSessionId`
+      // would always succeed (dispatch vs dispatch) — that was the
+      // first wiring bug acceptance caught. The live context is
+      // resolved from the MODULE-scope store snapshot (lib/store.tsx
+      // #getActiveSessionId), not from a per-instance ref. The
+      // composer's `submit` can outlive its own React tree —
+      // page.tsx swaps the composer between two positions when
+      // `hasConversation` flips, and creating a fresh session
+      // clears it. An instance-scoped ref frozen at dispatch time
+      // never sees the rotation; the module snapshot is the same
+      // store the SSE handler writes, so it always reflects the
+      // current session. `clientId()` is module-scope too
+      // (lib/cid.ts), so the cid side has always been correct.
+      const liveCid = clientId();
+      const liveSessionId = getActiveSessionId();
+      // failComposerSent returns the restore payload only when the
+      // LIVE context still matches the dispatch context — a session
+      // switch mid-flight must never paste the old session's text
+      // into the new session's composer.
+      const restored = failComposerSent({
+        cid: liveCid,
+        sessionId: liveSessionId,
+        error: errorMessage,
+      });
+      // Always set the error banner — the failure is real even when
+      // the active session no longer matches the record (the banner
+      // is in the module-scope draft store too, so it outlives a
+      // session switch).
+      if (restored) {
+        // The user may have typed INTERIM text during the in-flight
+        // window. We must not clobber it — "Nothing may vanish"
+        // applies to both the failed message and whatever the user
+        // typed since. Merge: put the restored text after the
+        // current draft with a blank-line separator. The error
+        // banner explains why the original bounced; both messages
+        // remain editable.
+        const current = getComposerDraft();
+        const interim = current.value.trim();
+        const mergedValue =
+          interim.length > 0
+            ? `${current.value}\n\n${restored.content}`
+            : restored.content;
+        // Restored attachments come first so the chip list reads in
+        // the order the user assembled it (the failed message's
+        // attachments, then any new attachments added meanwhile).
+        const mergedAttachments = [
+          ...restored.attachments,
+          ...current.attachments,
+        ];
+        setComposerDraft({ value: mergedValue, attachments: mergedAttachments });
+      }
+      setComposerDraft({ error: errorMessage });
     } finally {
       setSending(false);
     }
-  }, [value, attachments, readOnly, sending]);
+  }, [value, attachments, readOnly, sending, state?.sessionId]);
 
   const onPickFiles = useCallback(async (files: FileList | null) => {
     if (!files?.length) return;

packages/webui/webapp/lib/composer-sent.ts

@@ -0,0 +1,221 @@
+/**
+ * Composer outbox — the in-flight send record.
+ *
+ * Shaped like the draft store in `composer-draft.ts`: a single module-scope
+ * object, a `Set` of listeners, and a `subscribe`/`get` pair that
+ * `useSyncExternalStore` consumes. The store survives the composer
+ * remount that `page.tsx` triggers when the first user line arrives (a
+ * `useState`-held record would die with the unmounted instance), so the
+ * outbox is reachable both during a long in-flight window and after the
+ * user navigates back into the composer.
+ *
+ * Why a separate store instead of extending `composer-draft.ts`: the
+ * draft store is *what the user is typing right now*; the outbox is
+ * *what was just sent*. Mixing them would re-introduce the very
+ * vanishing-on-remount bug the draft store exists to prevent — a
+ * remount mid-send that re-uses the same store would lose either the
+ * draft or the record depending on which write landed last. Two
+ * independent module-scope stores keep the two concerns apart.
+ *
+ * Per-cid / per-session scoping: each record carries the `cid` and
+ * `sessionId` it was dispatched under. `failSend` only restores the
+ * text into the composer when both still match — switching sessions
+ * while a send is in flight MUST NOT paste another session's text into
+ * the new session's composer.
+ */
+
+export type ComposerSentStatus = "in-flight" | "delivered" | "failed";
+
+export interface ComposerSentRecord {
+  /** Browser-stable client id (`lib/cid.ts`). The per-cid server keeps
+   *  one engine subprocess; the outbox is keyed by it so two browsers
+   *  on the same machine never share a record. */
+  cid: string;
+  /** Active session at dispatch time (`state.sessionId`, possibly
+   *  null before the first snapshot). Session switches are caught by
+   *  matching on this field — a failure in session A must not be
+   *  restored into session B. */
+  sessionId: string | null;
+  /** Trimmed text the user submitted. Preserved verbatim for restore. */
+  content: string;
+  /** `@path` references captured at dispatch. Restored verbatim too. */
+  attachments: string[];
+  status: ComposerSentStatus;
+  /** Failure message from the rejected request; `null` while in-flight
+   *  or after a successful delivery. */
+  error: string | null;
+  /** `Date.now()` at the moment of the most recent state transition.
+   *  Test-only determinism: the reducer accepts an injected timestamp
+   *  so the same scenario is reproducible without a fake clock. */
+  timestamp: number;
+}
+
+// --- Pure reducers --------------------------------------------------------
+//
+// All branching in the composer is concentrated here so the rest of the
+// codebase (and the unit tests) can exercise the decision tree without a
+// browser, a fetch, or a React tree. The composer's `submit` callback
+// reads three transitions:
+//   1. dispatch    — start a new send (status -> in-flight).
+//   2. complete    — mark an in-flight record delivered.
+//   3. fail        — mark an in-flight record failed and (when cid +
+//                    sessionId still match) hand back the text the
+//                    composer should restore.
+
+/**
+ * Dispatch a fresh send.
+ *
+ * Attachments are deep-copied so a later mutation of the caller's
+ * array cannot leak into the record. The `timestamp` is overridable
+ * for tests; the store wrapper injects `Date.now()`.
+ */
+export function dispatchSend(args: {
+  cid: string;
+  sessionId: string | null;
+  content: string;
+  attachments: string[];
+  timestamp?: number;
+}): ComposerSentRecord {
+  return {
+    cid: args.cid,
+    sessionId: args.sessionId,
+    content: args.content,
+    attachments: [...args.attachments],
+    status: "in-flight",
+    error: null,
+    timestamp: args.timestamp ?? Date.now(),
+  };
+}
+
+/**
+ * Mark a record delivered.
+ *
+ * Idempotent only on `in-flight`: a `delivered` write is a no-op
+ * (already delivered), and a `failed` write is also a no-op so a
+ * late-arriving success signal after a failure cannot silently
+ * overwrite the failure banner. The composer calls this from the
+ * success branch of its try/catch; the test suite pins the no-op
+ * behaviour for the late-success case.
+ */
+export function completeSend(record: ComposerSentRecord): ComposerSentRecord {
+  if (record.status !== "in-flight") return record;
+  return { ...record, status: "delivered", error: null };
+}
+
+/** Restore payload returned by `failSend` when cid + sessionId match. */
+export interface RestorePayload {
+  content: string;
+  attachments: string[];
+}
+
+/**
+ * Mark an in-flight send failed and (when cid + sessionId match the
+ * record's) return the text + attachments to put back into the composer.
+ *
+ * Returns `null` when the dispatch context no longer matches — the
+ * user switched sessions, the cid rotated, etc. A `null` return means
+ * the record is left untouched; the failure is logged nowhere on the
+ * client because the wrong-session composer would have nothing to
+ * show it on.
+ *
+ * `null` is also returned when there is no record at all (defensive:
+ * the store wrapper already short-circuits, but the pure reducer has
+ * to behave identically for direct callers).
+ */
+export function failSend(
+  record: ComposerSentRecord | null,
+  args: {
+    cid: string;
+    sessionId: string | null;
+    error: string;
+    timestamp?: number;
+  },
+): { record: ComposerSentRecord; payload: RestorePayload } | null {
+  if (!record) return null;
+  if (record.cid !== args.cid) return null;
+  if (record.sessionId !== args.sessionId) return null;
+  const next: ComposerSentRecord = {
+    ...record,
+    status: "failed",
+    error: args.error,
+    timestamp: args.timestamp ?? Date.now(),
+  };
+  return {
+    record: next,
+    payload: { content: record.content, attachments: [...record.attachments] },
+  };
+}
+
+/** True iff the record's cid + sessionId match the active context. */
+export function recordMatches(
+  record: ComposerSentRecord | null,
+  cid: string,
+  sessionId: string | null,
+): boolean {
+  if (!record) return false;
+  return record.cid === cid && record.sessionId === sessionId;
+}
+
+// --- Store -----------------------------------------------------------------
+//
+// One record, last write wins. Only one send is ever in flight per
+// composer instance (the `sending` flag is the gate); a second submit
+// while the first is unresolved overwrites the previous record, which
+// is the right behaviour — the second message is the one the user is
+// now committed to sending.
+
+const listeners = new Set<() => void>();
+let record: ComposerSentRecord | null = null;
+
+export function startComposerSent(args: {
+  cid: string;
+  sessionId: string | null;
+  content: string;
+  attachments: string[];
+}): ComposerSentRecord {
+  record = dispatchSend(args);
+  for (const listener of listeners) listener();
+  return record;
+}
+
+export function completeComposerSent(): void {
+  if (!record || record.status !== "in-flight") return;
+  record = completeSend(record);
+  for (const listener of listeners) listener();
+}
+
+/**
+ * Mark the in-flight record failed. Returns the restore payload when
+ * the dispatch context still matches; otherwise `null` and the record
+ * is left as-is (the failure belongs to a session the user is no
+ * longer looking at).
+ */
+export function failComposerSent(args: {
+  cid: string;
+  sessionId: string | null;
+  error: string;
+}): RestorePayload | null {
+  const result = failSend(record, args);
+  if (!result) return null;
+  record = result.record;
+  for (const listener of listeners) listener();
+  return result.payload;
+}
+
+export function getComposerSent(): ComposerSentRecord | null {
+  return record;
+}
+
+/** `useSyncExternalStore` subscription. Returns the unsubscribe thunk. */
+export function subscribeComposerSent(listener: () => void): () => void {
+  listeners.add(listener);
+  return () => {
+    listeners.delete(listener);
+  };
+}
+
+/** Test-only: clear the store and its listeners between cases. */
+export function resetComposerSentForTests(): void {
+  record = null;
+  listeners.clear();
+}

packages/webui/webapp/lib/store.tsx

@@ -110,6 +110,29 @@ export function __testSnapshot(): StoreSnapshot {
   return snapshot;
 }
 
+/**
+ * Active session id from the module-scope snapshot.
+ *
+ * Why a separate accessor instead of reading from a React component's
+ * closure: the SSE-driven store lives at module scope (it must — the
+ * SSE connection has to outlive every component). A composer's
+ * in-flight submit can be closed over a ref that was current at
+ * dispatch time; if the user switches sessions, that ref is frozen
+ * at the OLD session id because the component may have remounted
+ * (page.tsx swaps the composer between the inline and chat-tree
+ * positions when `hasConversation` flips). Reading from this
+ * accessor at catch time resolves the live session id from the
+ * SAME module-scope state the SSE handler writes — it survives
+ * remounts and is updated by every state push.
+ *
+ * The composer's `submit` reads this at catch time so the cid +
+ * sessionId restore-gate compares against the active context, not
+ * a stale closure snapshot.
+ */
+export function getActiveSessionId(): string | null {
+  return snapshot.state ? snapshot.state.sessionId : null;
+}
+
 /**
  * Test-only handle: simulate an SSE frame dispatch. Production code
  * NEVER calls this — it exists so the revision-guard reducer can be