fix(chat): preserve screenshot/attachments when a send isn't delivered (#615)

用户报告:对话框发送截图后,截图直接消失且未真正发出。根因有三层,逐层修复,
其中 2、3 层由 Codex 真机 UI 冒烟发现并驱动:

1) onSend 投递契约。`onSend` 从 `=> void` 升级为 `=> boolean | void | Promise<…>`:
   返回 false = 未被接受投递(provider 加载中 / 无兼容 provider / runtime 不兼容 /
   禁用)→ composer 保留文字+附件;true/void = 已接受(发出或入队)→ 清空。
   - MessageInput:带附件的 no-send 路径(badge+streaming / disabled / 两处 onSend
     gated)统一走新 helper abortComposerSubmit(throw → PromptInput reject 分支不
     清空),不再裸 return;clears 移到 delivered 判定之后;run-checkpoint 块并入该
     helper。
   - ChatView.sendMessage:三个丢消息门(provider idle / noCompatible /
     runtime-incompatible)返回 false;入队与成功保持已投递。
   - page.tsx sendFirstMessage:5 个门返回 false。
   - useAssistantTrigger:sendMessageRef 类型放宽(自动触发忽略返回值)。

2) pre-delivery 失败(Codex 冒烟一)。session 创建 500 / POST /api/chat 被拒时 catch
   原来只设 banner、没回传未投递。新增 accepted 标记:仅 POST 2xx 后置真;catch 末尾
   if (!accepted) return false → 失败保留 composer。

3) remount 根因(Codex 冒烟二)。isNewChat 布局三元把 composer 渲染在两个不同父容器
   (居中 hero / 底部布局);旧代码点发送即 setIsStreaming(true) 翻转 isNewChat →
   composer 跨父 remount → PromptInput 本地附件 state 销毁,早于得知失败(稳定 key
   修不了跨父 remount)。改:把 setIsStreaming / 乐观气泡等驱动布局的翻转全部延后到
   accepted 之后;加 firstSendInFlightRef 双提交守卫;composer 栈 siblings 加稳定 key
   兜 ErrorBanner 同父重建。

测试:新增 composer-send-preservation.test.ts(source-pin 三文件的 no-send /
accepted / remount 契约);run-checkpoint-blocking.test.ts 的 throw 断言适配 helper。
triage 文档标记 #615 已修、Ollama/模型列表/串会话本轮不做(待复现)。

验证:tsc 0 错误;单元全量 3274 通过。正常路径带图发送经 Codex 冒烟确认
(/api/chat body 含 files[0]);两条注入失败路径由 Codex 冒烟驱动修复。
注:happy-path 的"发送中"指示因延后翻转会晚一两拍,为有意取舍。

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>

Author: op7418

docs/exec-plans/active/post-0.55.1-issue-triage.md

@@ -10,11 +10,11 @@
 | Phase | 内容 | 状态 | 备注 |
 |-------|------|------|------|
 | Phase 0 | Issue inventory + release/commit 对照 | ✅ 完成 | 覆盖 GitHub #606/#612/#613/#614/#615/#616/#617/#618/#619/#620/#621，以及 0.55 后更新的 #554/#577 |
-| Phase 1 | #615/#614 截图发送被吞 | 📋 待 Claude Code 修 | P0，源码已确认还有多条 no-send 成功返回路径会清空附件 |
-| Phase 2 | #620 Ollama 添加时要求安装 Claude Code | 📋 待 Claude Code 修 | P1，后端 provider test 不依赖 Claude Code，UI/语义提示疑似误导 |
-| Phase 3 | #613/#615 Codex/Opus 模型切换与模型列表不稳定 | 📋 待 Claude Code 深挖 | P1，本地 warm-cache 未复现；冷缓存/发现链路有源码风险 |
-| Phase 4 | #619 概率串会话 | 📋 待专项复现 | P1，高严重但当前未找到主路径 sessionId 分桶错误 |
-| Phase 5 | 低优先级/待补证队列 | 📋 待排期 | #612/#554/#617/#618/#606/#616/#621 |
+| Phase 1 | #615/#614 截图发送被吞 | ✅ 已实现（单测） | P0 已修：`onSend` 升级为可回传"未投递"的契约，MessageInput 所有带附件的 no-send 路径改走 `abortComposerSubmit`（首条 + 后续两条流都覆盖）。真实 UI smoke 待补 |
+| Phase 2 | #620 Ollama 添加时要求安装 Claude Code | ⏸ 本轮不做（用户 2026-06-08） | P1，方向较明确（语义/文案），但本轮先只做 P0；记录待排期 |
+| Phase 3 | #613/#615 Codex/Opus 模型切换与模型列表不稳定 | ⏸ 本轮不做（用户 2026-06-08） | P1，**未复现**，需冷缓存专项复现后再动，不盲改 |
+| Phase 4 | #619 概率串会话 | ⏸ 本轮不做（用户 2026-06-08） | P1，**未复现**，必须先按复现脚本坐实再碰 stream/ context，不盲改 |
+| Phase 5 | 低优先级/待补证队列 | ⏸ 本轮不做（用户 2026-06-08） | #612/#554/#617/#618/#606/#616/#621 |
 
 ## 用户会看到什么
 
@@ -36,6 +36,8 @@
 - 2026-06-08：#615 拆成两个问题处理：截图/附件丢失为 P0，可从 `PromptInput` + `MessageInput` 清空契约直接证明；模型无法选中/列表不稳定并入 Phase 3。
 - 2026-06-08：#620 定为语义/UX P1，而不是“真实需要 Claude Code CLI”。后端 test provider 已明确绕过 Claude Code SDK subprocess。
 - 2026-06-08：#619 定为高严重待复现。主路径 stream subscription 按 `sessionId` 分桶，当前不能直接判定为全局广播 bug。
+- 2026-06-08：Claude Code 复核 triage——深挖确认 P0（#615）成立（MessageInput 多条裸 `return` no-send 路径 + `onSend` 为 `void` 导致 ChatView/首消息页的 provider 门早退后附件已被清空；首条与后续两条流都中招）；spot-check Phase 2（Ollama）代码证据成立；Phase 3/4 认同"未复现、需专项复现再动"。
+- 2026-06-08：用户决定**本轮只修 P0（#615）**，Phase 2/3/4/5 暂不做（不确定 / 待复现），保留本文档作记录。P0 已由 Claude Code 实现并通过单测；真实 UI smoke 待补。其余 Phase 待用户后续主动发起。
 
 ## Release / Commit 对照
 

src/__tests__/unit/composer-send-preservation.test.ts

@@ -0,0 +1,118 @@
+/**
+ * #615 — screenshots eaten on a no-op send. Generalizes the run-checkpoint
+ * preservation contract: the composer must keep the user's text + attachments
+ * whenever a submit is NOT actually delivered, not just on the checkpoint block.
+ *
+ * Two halves:
+ *   1. MessageInput routes EVERY file-carrying no-send branch through
+ *      abortComposerSubmit() (which throws → PromptInput's reject branch keeps
+ *      text/files) instead of a bare `return` (which resolves → PromptInput
+ *      clears). The normal + badge sends now AWAIT onSend and abort when it
+ *      reports the send was gated.
+ *   2. The onSend providers (ChatView.sendMessage for subsequent messages,
+ *      page.tsx sendFirstMessage for the first) RETURN false on every
+ *      provider/runtime gate, so MessageInput can tell a gated send from a real
+ *      one. Before this, onSend was `void` and those gates fired *after*
+ *      MessageInput had already let PromptInput clear the screenshot.
+ *
+ * Source-level pins (the send path is React-coupled; no renderer here). They
+ * also guard against a future no-send branch being written as a bare `return`.
+ */
+
+import { describe, it } from 'node:test';
+import assert from 'node:assert/strict';
+import { readFileSync } from 'node:fs';
+import path from 'node:path';
+
+const read = (rel: string) => readFileSync(path.resolve(__dirname, '../..', rel), 'utf8');
+const countMatches = (src: string, re: RegExp) => (src.match(re) ?? []).length;
+
+describe('MessageInput: no-send branches preserve the composer (#615)', () => {
+  const src = read('components/chat/MessageInput.tsx');
+
+  it('has the shared abortComposerSubmit() throw helper', () => {
+    assert.match(src, /function abortComposerSubmit\(reason: string\): never\s*\{\s*throw new Error\(reason\)/);
+  });
+
+  it('awaits onSend and aborts when the send was not delivered (normal + badge paths)', () => {
+    // Both file-carrying sends must await the delivery signal …
+    assert.ok(
+      countMatches(src, /const delivered = await onSend\(/g) >= 2,
+      'both the normal and badge send paths must await onSend for the delivery signal',
+    );
+    // … and abort (preserve) when it comes back false.
+    assert.ok(
+      countMatches(src, /if \(delivered === false\) abortComposerSubmit\('composer-send-not-delivered'\)/g) >= 2,
+      'each awaited send must preserve the composer when delivered === false',
+    );
+  });
+
+  it('badge-during-streaming and disabled-with-content preserve instead of bare return', () => {
+    assert.match(src, /if \(isStreaming\) abortComposerSubmit\('composer-badge-streaming'\)/);
+    assert.match(src, /if \(disabled\) abortComposerSubmit\('composer-disabled'\)/);
+  });
+
+  it('the old screenshot-eating bare returns are gone (no `|| disabled) return;`)', () => {
+    assert.doesNotMatch(src, /\|\| disabled\)\s*return;/);
+  });
+});
+
+describe('ChatView.sendMessage signals not-delivered on every provider gate (#615)', () => {
+  const src = read('components/chat/ChatView.tsx');
+
+  it('provider-feed-loading gate returns false', () => {
+    assert.match(src, /providerFetchState === 'idle'\)[\s\S]{0,200}return false/);
+  });
+
+  it('no-compatible-provider gate returns false', () => {
+    assert.match(src, /sendMessage suppressed: no provider compatible[\s\S]{0,120}return false/);
+  });
+
+  it('runtime-incompatible gate returns false', () => {
+    assert.match(src, /sendMessage suppressed: session provider not compatible[\s\S]{0,160}return false/);
+  });
+});
+
+describe('page.tsx sendFirstMessage signals not-delivered on its gates (#615)', () => {
+  const src = read('app/chat/page.tsx');
+
+  it('model-not-ready gate returns false', () => {
+    assert.match(src, /if \(!modelReady\) return false/);
+  });
+
+  it('no-compatible-provider gate returns false', () => {
+    assert.match(src, /if \(noCompatibleProvider\)[\s\S]{0,200}return false/);
+  });
+
+  it('cannot-send-with-current-provider gate returns false', () => {
+    assert.match(src, /if \(!canSendWithCurrentProvider\)[\s\S]{0,200}return false/);
+  });
+
+  it('pre-delivery failure (session create / POST rejected) preserves the composer via the accepted flag (#615 Codex smoke)', () => {
+    // Codex's real UI smoke: inject a 500 into POST /api/chat/sessions → error
+    // banner shows, text stays, but the screenshot was eaten because the catch
+    // only set a banner (no return false). The `accepted` flag fixes it: only
+    // true once the backend accepts the message; the catch returns false otherwise.
+    assert.match(src, /let accepted = false/);
+    assert.match(src, /accepted = true/);
+    assert.match(src, /if \(!accepted\) return false/);
+  });
+
+  it('defers the isStreaming flip until after accept so a pre-accept failure does not remount the composer (#615 remount fix)', () => {
+    // Root cause beyond the return value: the isNewChat ternary renders the
+    // composer under two different parents (centered hero vs active layout).
+    // Flipping isStreaming before accept swaps the parent → MessageInput remounts
+    // → PromptInput loses the attachment BEFORE we learn the send failed. So the
+    // layout-driving flip must happen only after `accepted = true`, guarded
+    // against double-submit by an in-flight ref. (Source-pin only — the real
+    // proof is Codex's inject-500 UI smoke; this just guards the structure.)
+    assert.match(src, /const firstSendInFlightRef = useRef\(false\)/);
+    assert.match(src, /if \(firstSendInFlightRef\.current\) return false/);
+    assert.equal(countMatches(src, /setIsStreaming\(true\)/g), 1, 'isStreaming must be flipped in exactly one place (deferred to post-accept)');
+    assert.match(src, /accepted = true;[\s\S]{0,400}setIsStreaming\(true\)/);
+  });
+
+  it('composer-stack siblings are keyed so an ErrorBanner toggle keeps MessageInput identity (#615)', () => {
+    assert.match(src, /<MessageInput\s+key="composer-message-input"/);
+  });
+});

src/__tests__/unit/run-checkpoint-blocking.test.ts

@@ -127,7 +127,9 @@ describe('MessageInput ↔ PromptInput checkpoint preservation contract', () =>
 
   it('MessageInput throws the checkpoint-blocked sentinel instead of returning', () => {
     const src = read('components/chat/MessageInput.tsx');
-    assert.match(src, /throw new Error\(['"]run-checkpoint-blocked['"]\)/);
+    // #615 refactor: the raw throw moved into the shared abortComposerSubmit()
+    // helper. Accept either form so the sentinel stays pinned through the move.
+    assert.match(src, /(throw new Error|abortComposerSubmit)\(['"]run-checkpoint-blocked['"]\)/);
     assert.doesNotMatch(
       src,
       /blockingReasonIds[\s\S]{0,120}\{\s*return;\s*\}/,

src/app/chat/page.tsx

@@ -297,6 +297,12 @@ function NewChatPageInner() {
   }, []);
   const [createdSessionId, setCreatedSessionId] = useState<string | undefined>();
   const abortControllerRef = useRef<AbortController | null>(null);
+  // #615: guards the first-message send while it's mid-flight. We defer the
+  // isStreaming / optimistic-bubble flips until the backend ACCEPTS the message
+  // (otherwise flipping `isNewChat` remounts the composer and eats the
+  // screenshot), which means the usual `if (isStreaming) return` re-entry guard
+  // isn't armed during that window — this ref blocks a double-submit instead.
+  const firstSendInFlightRef = useRef(false);
   // Effort level — lifted here so the first message includes it
   const [selectedEffort, setSelectedEffort] = useState<string | undefined>(undefined);
   // Provider options (thinking mode + 1M context)
@@ -721,10 +727,13 @@ function NewChatPageInner() {
 
   const sendFirstMessage = useCallback(
     async (content: string, files?: FileAttachment[], systemPromptAppend?: string, displayOverride?: string, mentions?: MentionRef[], selectedSkills?: readonly string[]) => {
-      if (isStreaming) return;
+      // Each early-out below is a NOT-delivered case: return false so the
+      // composer preserves the user's text + attachments instead of letting
+      // PromptInput clear a first-message screenshot that never got sent (#615).
+      if (isStreaming) return false;
 
       // Wait for model/provider to be resolved from the global default before allowing send
-      if (!modelReady) return;
+      if (!modelReady) return false;
 
       // Block send when the runtime-filtered API returned an empty group
       // list — user has providers but none are compatible with the
@@ -737,7 +746,7 @@ function NewChatPageInner() {
           message: t('error.providerUnavailable'),
           description: t('chat.empty.noProvider'),
         });
-        return;
+        return false; // not delivered → preserve composer (#615)
       }
 
       // Phase 6 UI收口 P0 (2026-05-14): pinned-invalid is a GLOBAL
@@ -760,7 +769,7 @@ function NewChatPageInner() {
       // Require a project directory before sending
       if (!workingDir.trim()) {
         setErrorBanner({ message: t('chat.empty.noDirectory') });
-        return;
+        return false; // not delivered → preserve composer (#615)
       }
 
       // Phase 6 P0 follow-up (2026-05-15) — Codex Account is a virtual
@@ -783,19 +792,28 @@ function NewChatPageInner() {
           message: t('error.providerUnavailable'),
           description: t('chat.empty.noProvider'),
         });
-        return;
+        return false; // not delivered → preserve composer (#615)
       }
 
-      setIsStreaming(true);
-      setStreamingContent('');
-      setToolUses([]);
-      setToolResults([]);
-      setStatusText(undefined);
+      // #615 remount fix: do NOT flip isStreaming / push the optimistic bubble
+      // yet. Either flips `isNewChat` (messages.length === 0 && !isStreaming),
+      // which swaps the whole layout ternary — the composer moves from the
+      // centered hero branch to the active-layout branch (a DIFFERENT parent), so
+      // MessageInput remounts and PromptInput loses the attachment, BEFORE we even
+      // learn the send failed. Defer those flips to the post-accept point so a
+      // pre-acceptance failure leaves the hero (and the screenshot) untouched.
+      if (firstSendInFlightRef.current) return false; // double-submit guard while mid-flight
+      firstSendInFlightRef.current = true;
 
       const controller = new AbortController();
       abortControllerRef.current = controller;
 
       let sessionId = '';
+      // #615: tracks whether the message reached a delivered / recoverable state
+      // (session created + POST /api/chat accepted). A failure BEFORE this must
+      // return false so the composer preserves the user's text + attachments —
+      // otherwise a session-create 500 silently eats the screenshot.
+      let accepted = false;
 
       try {
         // Create a new session with working directory + model/provider
@@ -844,23 +862,10 @@ function NewChatPageInner() {
         // Notify ChatListPanel to refresh immediately
         window.dispatchEvent(new CustomEvent('session-created'));
 
-        // Add user message to UI — use displayOverride for chat bubble if provided
-        const displayUserContent = displayOverride || content;
-        // Optimistic save preserves base64 `data` so images can render
-        // their thumbnail immediately (see ChatView for the full
-        // explanation; backend still strips `data` before persistence).
-        const contentWithFileMeta = files && files.length > 0
-          ? `<!--files:${JSON.stringify(files.map(f => ({ id: f.id, name: f.name, type: f.type, size: f.size, data: f.data })))}-->${displayUserContent}`
-          : displayUserContent;
-        const userMessage: Message = {
-          id: 'temp-' + Date.now(),
-          session_id: session.id,
-          role: 'user',
-          content: contentWithFileMeta,
-          created_at: new Date().toISOString(),
-          token_usage: null,
-        };
-        setMessages([userMessage]);
+        // NOTE: the optimistic user bubble is pushed AFTER the message is
+        // accepted (post-accept block below), not here — pushing it now would
+        // make messages non-empty → flip isNewChat → remount the composer and
+        // eat the screenshot on a /api/chat rejection. (#615)
 
         // Build thinking config from settings
         const thinkingConfig = thinkingMode && thinkingMode !== 'adaptive'
@@ -902,6 +907,37 @@ function NewChatPageInner() {
           }
           throw new Error(err?.error || 'Failed to send message');
         }
+        // Backend accepted the message + files (POST /api/chat is 2xx and the
+        // stream is opening) — from here the screenshot is committed
+        // server-side, so a later error must NOT preserve the composer (#615).
+        accepted = true;
+
+        // Flip the layout-driving state ONLY now: show streaming + push the
+        // optimistic user bubble. Deferring to here keeps `isNewChat` true
+        // through any pre-acceptance failure, so the composer never remounts and
+        // the screenshot survives (#615).
+        setIsStreaming(true);
+        setStreamingContent('');
+        setToolUses([]);
+        setToolResults([]);
+        setStatusText(undefined);
+        {
+          // Optimistic user bubble — preserves base64 `data` so images render
+          // their thumbnail immediately (backend strips `data` before persisting).
+          const displayUserContent = displayOverride || content;
+          const contentWithFileMeta = files && files.length > 0
+            ? `<!--files:${JSON.stringify(files.map(f => ({ id: f.id, name: f.name, type: f.type, size: f.size, data: f.data })))}-->${displayUserContent}`
+            : displayUserContent;
+          const userMessage: Message = {
+            id: 'temp-' + Date.now(),
+            session_id: session.id,
+            role: 'user',
+            content: contentWithFileMeta,
+            created_at: new Date().toISOString(),
+            token_usage: null,
+          };
+          setMessages([userMessage]);
+        }
 
         const reader = response.body?.getReader();
         if (!reader) throw new Error('No response stream');
@@ -1115,6 +1151,11 @@ function NewChatPageInner() {
           const errMsg = error instanceof Error ? error.message : 'Unknown error';
           setErrorBanner({ message: t('error.sessionCreateFailed'), description: errMsg });
         }
+        // #615: a failure BEFORE the message was accepted for delivery (session
+        // creation or POST /api/chat rejected) must preserve the composer so the
+        // user's screenshot isn't cleared. Post-acceptance errors (mid-stream)
+        // keep today's behavior — the message already went, so the composer clears.
+        if (!accepted) return false;
       } finally {
         setIsStreaming(false);
         setStreamingContent('');
@@ -1127,6 +1168,7 @@ function NewChatPageInner() {
         setPermissionResolved(null);
         setPendingApprovalSessionId('');
         abortControllerRef.current = null;
+        firstSendInFlightRef.current = false;
       }
     },
     [isStreaming, router, workingDir, mode, currentModel, currentProviderId, permissionProfile, selectedEffort, thinkingMode, context1m, setPendingApprovalSessionId, t, canSendWithCurrentProvider, modelReady, noCompatibleProvider, invalidDefault]
@@ -1210,8 +1252,14 @@ function NewChatPageInner() {
   // ChatComposerActionBar across two branches.
   const composerStack = (
     <>
+      {/* #615: stable keys so MessageInput keeps its identity (and PromptInput
+          keeps its attachment state) when ErrorBanner appears/disappears as a
+          sibling. The dominant remount cause — the isNewChat layout swap — is
+          fixed by deferring the layout-flip until accept (see sendFirstMessage);
+          these keys cover the within-parent ErrorBanner toggle. */}
       {errorBanner && (
         <ErrorBanner
+          key="composer-error-banner"
           message={errorBanner.message}
           description={errorBanner.description}
           className="mx-4 mb-2"
@@ -1221,14 +1269,16 @@ function NewChatPageInner() {
           ]}
         />
       )}
-      <RunCheckpoint reasons={checkpointReasons} className="mb-2" onAction={handleCheckpointAction} />
+      <RunCheckpoint key="composer-run-checkpoint" reasons={checkpointReasons} className="mb-2" onAction={handleCheckpointAction} />
       <PermissionPrompt
+        key="composer-permission-prompt"
         pendingPermission={pendingPermission}
         permissionResolved={permissionResolved}
         onPermissionResponse={handlePermissionResponse}
         toolUses={toolUses}
       />
       <MessageInput
+        key="composer-message-input"
         onSend={sendFirstMessage}
         onCommand={handleCommand}
         onStop={stopStreaming}