From efe222af03bd4a078633aea6047d364169311f08 Mon Sep 17 00:00:00 2001 From: Cursor Agent Date: Fri, 6 Nov 2026 15:31:24 +0000 Subject: [PATCH] fix(web): keep a side chat's first question if the panel closes during send The unused-delete guard was noted only inside startThreadTurn, after the title, settings, and attachment round trips. Closing the panel in that window deleted the side chat before the turn was submitted, so the question could not be reopened. Note the send when it is committed. Co-Authored-By: mubeda <1380447+mubeda@users.noreply.github.com> --- apps/web/src/components/ChatView.test.tsx | 49 ++++++++++++++++++++++ apps/web/src/components/ChatView.tsx | 11 ++++- docs/architecture/rpc-and-orchestration.md | 7 ++-- 3 files changed, 63 insertions(+), 4 deletions(-) diff --git a/apps/web/src/components/ChatView.test.tsx b/apps/web/src/components/ChatView.test.tsx index da768b9be9..1266f3008d 100644 --- a/apps/web/src/components/ChatView.test.tsx +++ b/apps/web/src/components/ChatView.test.tsx @@ -81,6 +81,8 @@ const h = vi.hoisted(() => { atomValuesByKey: new Map(), atomListeners: new Set<() => void>(), commandCalls: [] as Array<{ key: string; input: unknown }>, + /** When set, `readPreparedConnection` returns this instead of the live session. */ + preparedConnection: null as { descriptor: { capabilities: { threadDeleteOnlyIfUnused: boolean } } } | null, commandHandlers: new Map Promise>(), commandOptions: [] as Array<{ key: string; options: unknown }>, commandResults: {} as Record unknown>, @@ -363,8 +365,18 @@ vi.mock("../state/entities", () => ({ ref ? (h.threadsByKey.get(`${ref.environmentId}:${ref.threadId}`) ?? null) : null, useThreadRefs: () => h.threadRefs, useThreadProposedPlans: () => [], + readThreadShell: () => null, })); +vi.mock("../state/session", async (importOriginal) => { + const actual = await importOriginal(); + return { + ...actual, + readPreparedConnection: (environmentId: EnvironmentId) => + h.preparedConnection ?? actual.readPreparedConnection(environmentId), + }; +}); + vi.mock("../state/environments", () => ({ useEnvironments: () => ({ isReady: true, @@ -819,6 +831,7 @@ vi.mock("./activity/ActivityPanel", async (importOriginal) => { }); import ChatView from "./ChatView"; +import { closesByUnusedDelete } from "../centerPanelClose"; import { ActivityPanelBinding } from "./activity/ActivityPaneBinding"; import type { Project, Thread } from "../types"; import { useComposerDraftStore } from "../composerDraftStore"; @@ -1089,6 +1102,7 @@ beforeEach(() => { h.atomValuesByKey.set("atom:keybindings", []); h.atomValuesByKey.set("atom:editors", []); h.commandCalls.length = 0; + h.preparedConnection = null; h.commandHandlers.clear(); h.commandOptions.length = 0; h.commandResults = { @@ -5776,6 +5790,41 @@ describe("ChatView handlers (captured from mocked children)", () => { ).toBeUndefined(); }); + it("notes a first send before the title round trip so closing cannot delete it", async () => { + const raceThreadId = ThreadId.make("side-send-race"); + const raceRef = scopeThreadRef(environmentId, raceThreadId); + h.preparedConnection = { + descriptor: { capabilities: { threadDeleteOnlyIfUnused: true } }, + }; + seedConnectedServerThread(makeThread({ id: raceThreadId })); + renderToStaticMarkup( + , + ); + const composer = capturedProps>("chatComposer"); + const composerRef = composer["composerRef"] as RefObject; + composerRef.current = composerHandle(); + const promptRef = composer["promptRef"] as RefObject; + promptRef.current = "why that file?"; + expect(closesByUnusedDelete(raceRef)).toBe(true); + + const title = deferredResult>>(); + h.commandResults["thread.updateMetadata"] = () => title.promise; + const sending = (composer["onSend"] as () => Promise)(); + + expect(commandCallsFor("thread.updateMetadata").length).toBeGreaterThan(0); + expect(commandCallsFor("thread.startTurn")).toHaveLength(0); + expect(closesByUnusedDelete(raceRef)).toBe(false); + + title.resolve(AsyncResult.success(undefined)); + await sending; + expect(commandCallsFor("thread.startTurn")).toHaveLength(1); + }); + it("keeps an exact session account in the label, picker guard, and outgoing turn", async () => { const customInstanceId = ProviderInstanceId.make("codex_personal"); const startedThread = makeThread({ diff --git a/apps/web/src/components/ChatView.tsx b/apps/web/src/components/ChatView.tsx index 987ebcfe92..5be42e11f8 100644 --- a/apps/web/src/components/ChatView.tsx +++ b/apps/web/src/components/ChatView.tsx @@ -1099,7 +1099,8 @@ function ChatViewContent(props: ChatViewProps) { const startThreadTurnCommand = useAtomCommand(threadEnvironment.startTurn, { reportFailure: false, }); - // Every send is noted first, so closing an unused side chat while it goes out archives it. + // Noted again at the turn call, including the attachment replay below. onSend and + // plan follow-up note earlier: title, settings, and uploads run before this call. const startThreadTurn = useCallback( (value, runOptions) => { markChatPanelSent(scopeThreadRef(value.environmentId, value.input.threadId)); @@ -5748,6 +5749,11 @@ function ChatViewContent(props: ChatViewProps) { const messageIdForSend = newMessageId(); sendInFlightRef.current = true; + // Before title, settings, and attachment round trips. A close during those + // awaits must archive an unused side chat, not delete the question with it. + if (isServerThread) { + markChatPanelSent(scopeThreadRef(environmentId, threadIdForSend)); + } if (!enqueue && composerAttachments.length === 0) beginLocalDispatch({ preparingWorktree: Boolean(baseBranchForWorktree), @@ -6543,6 +6549,9 @@ function ChatViewContent(props: ChatViewProps) { }); sendInFlightRef.current = true; + // Settings are saved before the turn. Note the send first so a close during + // that round trip archives an unused side chat instead of deleting it. + markChatPanelSent(scopeThreadRef(activeThread.environmentId, threadIdForSend)); beginLocalDispatch({ preparingWorktree: false, threadId: threadIdForSend, diff --git a/docs/architecture/rpc-and-orchestration.md b/docs/architecture/rpc-and-orchestration.md index ffeff1700d..647fc8d52b 100644 --- a/docs/architecture/rpc-and-orchestration.md +++ b/docs/architecture/rpc-and-orchestration.md @@ -2240,9 +2240,10 @@ command replays as rejected and never deletes. The Rust field is omitted when unknown fields, so an older server would ignore `onlyIfUnused` and delete a used side chat: the client sends it only to a server that advertises `capabilities.threadDeleteOnlyIfUnused`, and only for a side chat whose shell -shows no turn or user message and that this page has not sent to (every -`thread.turn.start` from `ChatView` and the `:btw` send note it first). A -refusal archives the side chat through the normal close (interrupt, then +shows no turn or user message and that this page has not sent to. A committed +send in `ChatView` is noted before the title, settings, and attachment round +trips that precede `thread.turn.start`, and the `:btw` send notes it before its +turn. A refusal archives the side chat through the normal close (interrupt, then `thread.archive`); an interrupted delete is uncertain, as an interrupted archive is. A delete settles the close as an archive does: the shell that reaches its sequence no longer lists the panel, and it never reaches **Reopen