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