From d74747fbf06396865ccdfafeffdcfc0e72f72eed Mon Sep 17 00:00:00 2001 From: murashit Date: Tue, 21 Jul 2026 14:12:26 +0900 Subject: [PATCH] fix(threads): serialize rename saves --- src/domain/threads/rename-lifecycle.ts | 22 ++++- .../chat/application/state/ui-state.ts | 27 ++++++ .../threads/rename-editor-actions.ts | 36 ++++---- .../chat/panel/surface/toolbar-projection.tsx | 10 +- src/features/chat/ui/toolbar.tsx | 23 ++--- src/features/threads-view/session.ts | 42 +++++---- src/features/threads-view/shell.dom.tsx | 15 +-- src/features/threads/list/row-projection.ts | 2 + tests/domain/threads/rename-lifecycle.test.ts | 19 ++++ .../threads/rename-editor-actions.test.ts | 18 ++-- .../chat/panel/surface/projections.test.ts | 32 +++++++ tests/features/chat/ui/toolbar.test.ts | 92 ++++++++++++++++--- tests/features/threads-view/shell.test.tsx | 32 ++++++- tests/features/threads-view/view.test.ts | 24 ++--- .../threads/list/row-projection.test.ts | 1 + 15 files changed, 297 insertions(+), 98 deletions(-) diff --git a/src/domain/threads/rename-lifecycle.ts b/src/domain/threads/rename-lifecycle.ts index 03bb6bdb..378e8903 100644 --- a/src/domain/threads/rename-lifecycle.ts +++ b/src/domain/threads/rename-lifecycle.ts @@ -5,16 +5,21 @@ type ThreadRenameAutoNameState = { kind: "checking" } | { kind: "unavailable" } export type ThreadRenameLifecycleState = | { kind: "idle" } | { kind: "editing"; draft: string; autoName: ThreadRenameAutoNameState } + | { kind: "saving"; draft: string; autoName: ThreadRenameAutoNameState; saveToken: number } | { kind: "generating"; draft: string; autoName: Extract; generationToken: number }; export type ThreadRenameActiveState = Exclude; type ThreadRenameGeneratingState = Extract; +type ThreadRenameSavingState = Extract; export type ThreadRenameLifecycleEvent = | { type: "started"; draft: string } | { type: "draft-updated"; draft: string } | { type: "auto-name-context-resolved"; context: ThreadTitleContext | null } | { type: "cancelled" } + | { type: "save-started"; saveToken: number } + | { type: "save-failed"; saveToken: number } + | { type: "save-succeeded"; saveToken: number } | { type: "generation-started"; generationToken: number } | { type: "generation-succeeded"; generationToken: number; draft: string } | { type: "generation-finished"; generationToken: number } @@ -30,17 +35,26 @@ export function transitionThreadRenameLifecycleState( ): ThreadRenameLifecycleState { switch (event.type) { case "started": + if (state.kind === "saving") return state; return { kind: "editing", draft: event.draft, autoName: { kind: "checking" } }; case "draft-updated": return state.kind === "editing" ? { ...state, draft: event.draft } : state; case "auto-name-context-resolved": - if (state.kind !== "editing") return state; + if (state.kind !== "editing" && state.kind !== "saving") return state; return { ...state, autoName: event.context ? { kind: "ready", context: event.context } : { kind: "unavailable" }, }; case "cancelled": - return state.kind === "idle" ? state : initialThreadRenameLifecycleState(); + return state.kind === "saving" || state.kind === "idle" ? state : initialThreadRenameLifecycleState(); + case "save-started": + return state.kind === "editing" ? { ...state, kind: "saving", saveToken: event.saveToken } : state; + case "save-failed": + return threadRenameSaveStillActive(state, event.saveToken) + ? { kind: "editing", draft: state.draft, autoName: state.autoName } + : state; + case "save-succeeded": + return threadRenameSaveStillActive(state, event.saveToken) ? initialThreadRenameLifecycleState() : state; case "generation-started": if (state.kind !== "editing" || state.autoName.kind !== "ready") return state; return { @@ -69,6 +83,10 @@ export function threadRenameGenerationStillActive( return state.kind === "generating" && state.generationToken === generationToken; } +export function threadRenameSaveStillActive(state: ThreadRenameLifecycleState, saveToken: number): state is ThreadRenameSavingState { + return state.kind === "saving" && state.saveToken === saveToken; +} + function unhandledThreadRenameLifecycleEvent(event: never): never { throw new Error(`Unhandled thread rename lifecycle event: ${String(event)}`); } diff --git a/src/features/chat/application/state/ui-state.ts b/src/features/chat/application/state/ui-state.ts index 4c7e3a54..ec5c72a5 100644 --- a/src/features/chat/application/state/ui-state.ts +++ b/src/features/chat/application/state/ui-state.ts @@ -5,6 +5,7 @@ import { type ThreadRenameLifecycleEvent, type ThreadRenameLifecycleState, threadRenameGenerationStillActive, + threadRenameSaveStillActive, transitionThreadRenameLifecycleState, } from "../../../../domain/threads/rename-lifecycle"; import type { ThreadTitleContext } from "../../../../domain/threads/title-generation-model"; @@ -14,6 +15,7 @@ import { patchObject } from "./patch"; export type ChatRenameUiState = { readonly kind: "idle" } | (ThreadRenameActiveState & { readonly threadId: string }); export type ChatRenameGeneratingUiState = Extract; +export type ChatRenameSavingUiState = Extract; type ChatRenameUiAction = Extract< UiAction, { @@ -22,6 +24,9 @@ type ChatRenameUiAction = Extract< | "ui/rename-draft-updated" | "ui/rename-auto-name-context-resolved" | "ui/rename-cancelled" + | "ui/rename-save-started" + | "ui/rename-save-failed" + | "ui/rename-save-succeeded" | "ui/rename-generation-started" | "ui/rename-generation-succeeded" | "ui/rename-generation-finished" @@ -75,6 +80,9 @@ export type UiAction = | { type: "ui/rename-draft-updated"; threadId: string; draft: string } | { type: "ui/rename-auto-name-context-resolved"; threadId: string; context: ThreadTitleContext | null } | { type: "ui/rename-cancelled"; threadId: string } + | { type: "ui/rename-save-started"; threadId: string; saveToken: number } + | { type: "ui/rename-save-failed"; threadId: string; saveToken: number } + | { type: "ui/rename-save-succeeded"; threadId: string; saveToken: number } | { type: "ui/rename-generation-started"; threadId: string; generationToken: number } | { type: "ui/rename-generation-succeeded"; threadId: string; generationToken: number; draft: string } | { type: "ui/rename-generation-finished"; threadId: string; generationToken: number } @@ -104,6 +112,9 @@ export function isUiAction(action: { type: string }): action is UiAction { case "ui/rename-draft-updated": case "ui/rename-auto-name-context-resolved": case "ui/rename-cancelled": + case "ui/rename-save-started": + case "ui/rename-save-failed": + case "ui/rename-save-succeeded": case "ui/rename-generation-started": case "ui/rename-generation-succeeded": case "ui/rename-generation-finished": @@ -129,6 +140,9 @@ export function reduceUiSlice(state: ChatUiState, action: UiAction): ChatUiState case "ui/rename-draft-updated": case "ui/rename-auto-name-context-resolved": case "ui/rename-cancelled": + case "ui/rename-save-started": + case "ui/rename-save-failed": + case "ui/rename-save-succeeded": case "ui/rename-generation-started": case "ui/rename-generation-succeeded": case "ui/rename-generation-finished": @@ -177,6 +191,10 @@ export function renameGenerationStillActive( return state.kind === "generating" && state.threadId === threadId && threadRenameGenerationStillActive(state, generationToken); } +export function renameSaveStillActive(state: ChatRenameUiState, threadId: string, saveToken: number): state is ChatRenameSavingUiState { + return state.kind === "saving" && state.threadId === threadId && threadRenameSaveStillActive(state, saveToken); +} + export function clearAllRequestDisclosures(state: ChatUiState): ChatUiState { if (state.disclosures.approvalDetails.size === 0) return state; return patchObject(state, { @@ -258,6 +276,7 @@ function goalEditorDraftUpdated(state: ChatGoalEditorUiState, objective: string) function transitionChatRenameUiState(state: ChatRenameUiState, action: ChatRenameUiAction): ChatRenameUiState { switch (action.type) { case "ui/rename-started": + if (state.kind === "saving") return state; return { kind: "editing", threadId: action.threadId, draft: action.draft, autoName: { kind: "checking" } }; case "ui/rename-draft-updated": return transitionScopedChatRenameUiState(state, action.threadId, { type: "draft-updated", draft: action.draft }); @@ -268,6 +287,12 @@ function transitionChatRenameUiState(state: ChatRenameUiState, action: ChatRenam }); case "ui/rename-cancelled": return transitionScopedChatRenameUiState(state, action.threadId, { type: "cancelled" }); + case "ui/rename-save-started": + return transitionScopedChatRenameUiState(state, action.threadId, { type: "save-started", saveToken: action.saveToken }); + case "ui/rename-save-failed": + return transitionScopedChatRenameUiState(state, action.threadId, { type: "save-failed", saveToken: action.saveToken }); + case "ui/rename-save-succeeded": + return transitionScopedChatRenameUiState(state, action.threadId, { type: "save-succeeded", saveToken: action.saveToken }); case "ui/rename-generation-started": return transitionScopedChatRenameUiState(state, action.threadId, { type: "generation-started", @@ -315,6 +340,8 @@ function chatRenameActiveStateWithoutThreadId(state: Exclude item.id === threadId); if (!thread) return; abortActiveGeneration(); @@ -71,6 +74,8 @@ export function createThreadRenameEditorActions(host: ThreadRenameEditorActionsH }, cancel(threadId: string): void { + const current = renameState(host); + if (current.kind === "saving" && current.threadId === threadId) return; abortGeneration(threadId); if (activeContextPreparation?.threadId === threadId) activeContextPreparation = null; dispatch(host, { type: "ui/rename-cancelled", threadId }); @@ -85,24 +90,22 @@ export function createThreadRenameEditorActions(host: ThreadRenameEditorActionsH async save(threadId: string, value: string): Promise { const current = renameState(host); - if (current.kind === "idle" || current.threadId !== threadId || current.kind === "generating") return; - const editingState = current; + if (current.kind !== "editing" || current.threadId !== threadId) return; + const saveToken = nextRenameSaveToken; + dispatch(host, { type: "ui/rename-save-started", threadId, saveToken }); + if (!renameSaveStillActive(renameState(host), threadId, saveToken)) return; + nextRenameSaveToken += 1; try { await host.ensureConnected(); - if (!renameEditStillCurrent(host, threadId, editingState.draft)) return; + if (!renameSaveStillActive(renameState(host), threadId, saveToken)) return; - const result = await host.renameThread(threadId, value); - if (!result) { - if (renameEditStillCurrent(host, threadId, editingState.draft)) action.cancel(threadId); - return; - } - if (renameEditStillCurrent(host, threadId, editingState.draft)) { - dispatch(host, { type: "ui/rename-cleared" }); - } + await host.renameThread(threadId, value); + dispatch(host, { type: "ui/rename-save-succeeded", threadId, saveToken }); } catch (error) { - if (!renameEditStillCurrent(host, threadId, editingState.draft)) return; + if (!renameSaveStillActive(renameState(host), threadId, saveToken)) return; host.addSystemMessage(error instanceof Error ? error.message : String(error)); + dispatch(host, { type: "ui/rename-save-failed", threadId, saveToken }); } }, @@ -168,7 +171,7 @@ export function createThreadRenameEditorActions(host: ThreadRenameEditorActionsH if (activeContextPreparation !== preparation) return; activeContextPreparation = null; const current = renameState(host); - if (current.kind !== "editing" || current.threadId !== threadId) return; + if ((current.kind !== "editing" && current.kind !== "saving") || current.threadId !== threadId) return; dispatch(host, { type: "ui/rename-auto-name-context-resolved", threadId, context }); } } @@ -181,11 +184,6 @@ function renameState(host: ThreadRenameEditorActionsHost): ChatRenameUiState { return host.stateStore.getState().ui.rename; } -function renameEditStillCurrent(host: ThreadRenameEditorActionsHost, threadId: string, draft: string): boolean { - const current = renameState(host); - return current.kind === "editing" && current.threadId === threadId && current.draft === draft; -} - function dispatch(host: ThreadRenameEditorActionsHost, action: ChatAction): void { host.stateStore.dispatch(action); } diff --git a/src/features/chat/panel/surface/toolbar-projection.tsx b/src/features/chat/panel/surface/toolbar-projection.tsx index 77bb5e03..d3b2ac17 100644 --- a/src/features/chat/panel/surface/toolbar-projection.tsx +++ b/src/features/chat/panel/surface/toolbar-projection.tsx @@ -206,13 +206,17 @@ function toolbarThreadRows(input: { title: core.title, threadId: core.threadId, selected: core.selected, - disabled: false, - openDisabled: false, + renameDisabled: input.renameState.kind === "saving", archiveDisabled: input.turnBusy, canArchive: true, archiveConfirm: core.archiveConfirm, rename: core.rename.active - ? { draft: core.rename.draft, generating: core.rename.generating, autoNameDisabled: core.rename.autoNameDisabled } + ? { + draft: core.rename.draft, + generating: core.rename.generating, + saving: core.rename.saving, + autoNameDisabled: core.rename.autoNameDisabled, + } : null, }; }); diff --git a/src/features/chat/ui/toolbar.tsx b/src/features/chat/ui/toolbar.tsx index bab9fffe..a94a4c96 100644 --- a/src/features/chat/ui/toolbar.tsx +++ b/src/features/chat/ui/toolbar.tsx @@ -12,14 +12,14 @@ export interface ToolbarThreadRow { title: string; threadId: string; selected: boolean; - disabled: boolean; - openDisabled?: boolean; + renameDisabled: boolean; archiveDisabled: boolean; canArchive: boolean; archiveConfirm?: { active: boolean; defaultSaveMarkdown: boolean }; rename: { draft: string; generating: boolean; + saving: boolean; autoNameDisabled: boolean; } | null; } @@ -411,7 +411,6 @@ function ThreadListRow({ thread, actions }: { thread: ToolbarThreadRow; actions: label={thread.title} selected={thread.selected} selectionStyle="row" - disabled={thread.openDisabled ?? thread.disabled} className="codex-panel__thread" onClick={() => { actions.resume(thread.threadId); @@ -422,7 +421,7 @@ function ThreadListRow({ thread, actions }: { thread: ToolbarThreadRow; actions: icon="pencil" label="Rename thread" className="codex-panel__thread-action" - disabled={thread.disabled} + disabled={thread.renameDisabled} onClick={(event) => { event.stopPropagation(); actions.rename.start(thread.threadId); @@ -490,10 +489,12 @@ function ArchiveModeButton({ function ThreadRenameRow({ thread, actions }: { thread: ToolbarThreadRow; actions: ToolbarThreadActions }): UiNode { const inputRef = useRef(null); const generating = thread.rename?.generating ?? false; + const saving = thread.rename?.saving ?? false; + const renameBusy = generating || saving; const draft = thread.rename?.draft ?? thread.title; useLayoutEffect(() => { - if (!generating) focusToolbarRenameInput(inputRef.current); - }, [draft, generating]); + if (!renameBusy) focusToolbarRenameInput(inputRef.current); + }, [draft, renameBusy]); return ( <> @@ -508,23 +509,23 @@ function ThreadRenameRow({ thread, actions }: { thread: ToolbarThreadRow; action className="codex-panel-ui__nav-inline-input codex-panel__thread-rename-input" type="text" value={draft} - disabled={generating} + disabled={renameBusy} onInput={(event) => { actions.rename.updateDraft(thread.threadId, event.currentTarget.value); }} onKeyDown={(event) => { if (event.key === "Enter") { event.preventDefault(); - if (!event.isComposing && !generating) actions.rename.save(thread.threadId, event.currentTarget.value); + if (!event.isComposing && !renameBusy) actions.rename.save(thread.threadId, event.currentTarget.value); return; } if (event.key === "Escape") { event.preventDefault(); - actions.rename.cancel(thread.threadId); + if (!renameBusy) actions.rename.cancel(thread.threadId); } }} onBlur={(event) => { - if (!generating) actions.rename.save(thread.threadId, event.currentTarget.value); + if (!renameBusy) actions.rename.save(thread.threadId, event.currentTarget.value); }} /> @@ -534,7 +535,7 @@ function ThreadRenameRow({ thread, actions }: { thread: ToolbarThreadRow; action icon={generating ? "x" : "sparkles"} label={generating ? "Cancel auto-name" : "Auto-name thread"} className="codex-panel__thread-action" - disabled={!generating && (thread.rename?.autoNameDisabled ?? true)} + disabled={saving || (!generating && (thread.rename?.autoNameDisabled ?? true))} onPointerDown={(event) => { event.preventDefault(); event.stopPropagation(); diff --git a/src/features/threads-view/session.ts b/src/features/threads-view/session.ts index 8ecb5daa..405c3abf 100644 --- a/src/features/threads-view/session.ts +++ b/src/features/threads-view/session.ts @@ -68,6 +68,7 @@ export class ThreadsViewSession { private readonly renameContextPreparations = new Map(); private readonly renameGenerationControllers = new Map(); private nextRenameGenerationToken = 1; + private nextRenameSaveToken = 1; private unsubscribeThreads: (() => void) | null = null; private archiveConfirmThreadId: string | null = null; @@ -261,6 +262,8 @@ export class ThreadsViewSession { } private startRename(threadId: string, value: string): void { + const current = this.renameStates.get(threadId); + if (current?.kind === "saving") return; this.archiveConfirmThreadId = null; this.transitionRenameState(threadId, { type: "started", draft: value }); this.render(); @@ -273,6 +276,7 @@ export class ThreadsViewSession { } private cancelRename(threadId: string): void { + if (this.renameStates.get(threadId)?.kind === "saving") return; this.abortAutoName(threadId); this.renameContextPreparations.delete(threadId); this.transitionRenameState(threadId, { type: "cancelled" }); @@ -288,24 +292,30 @@ export class ThreadsViewSession { private async saveRename(threadId: string, value: string): Promise { const lease = this.captureOperationLease(); - const editingState = this.renameStates.get(threadId); - if (!editingState || editingState.kind === "generating") return; + const previousState = this.renameStates.get(threadId); + if (previousState?.kind !== "editing") return; + const saveToken = this.nextRenameSaveToken; + const savingState = this.transitionRenameState(threadId, { type: "save-started", saveToken }); + if (savingState === previousState || savingState?.kind !== "saving") return; + this.nextRenameSaveToken += 1; + this.render(); try { - const result = await this.operations.renameThread(threadId, value, { - shouldStart: () => this.operationContextIsCurrent(lease), - shouldPublish: () => this.operationContextIsCurrent(lease), + await this.operations.renameThread(threadId, value, { + shouldStart: () => this.operationContextIsCurrent(lease) && this.renameSaveStillActive(threadId, saveToken), + shouldPublish: () => this.operationContextIsCurrent(lease) && this.renameSaveStillActive(threadId, saveToken), }); - if (!this.operationViewIsCurrent(lease) || !this.renameEditStillCurrent(threadId, editingState.draft)) return; - if (!result) { - this.cancelRename(threadId); - return; + if (!this.operationViewIsCurrent(lease)) return; + const currentState = this.renameStates.get(threadId); + const nextState = this.transitionRenameState(threadId, { type: "save-succeeded", saveToken }); + if (nextState !== currentState) { + if (!nextState) this.renameContextPreparations.delete(threadId); + this.render(); } - this.renameStates.delete(threadId); - this.renameContextPreparations.delete(threadId); - this.render(); } catch (error) { - if (!this.operationViewIsCurrent(lease) || !this.renameEditStillCurrent(threadId, editingState.draft)) return; + if (!this.operationViewIsCurrent(lease) || !this.renameSaveStillActive(threadId, saveToken)) return; this.noticeError(error); + this.transitionRenameState(threadId, { type: "save-failed", saveToken }); + this.render(); } } @@ -409,7 +419,7 @@ export class ThreadsViewSession { if (!this.lifetime.isActive() || this.renameContextPreparations.get(threadId) !== preparation) return; this.renameContextPreparations.delete(threadId); const state = this.renameStates.get(threadId); - if (state?.kind !== "editing") return; + if (state?.kind !== "editing" && state?.kind !== "saving") return; this.transitionRenameState(threadId, { type: "auto-name-context-resolved", context }); this.render(); } @@ -429,9 +439,9 @@ export class ThreadsViewSession { return nextState; } - private renameEditStillCurrent(threadId: string, draft: string): boolean { + private renameSaveStillActive(threadId: string, saveToken: number): boolean { const current = this.renameStates.get(threadId); - return current?.kind === "editing" && current.draft === draft; + return current?.kind === "saving" && current.saveToken === saveToken; } private viewWindow(): Window { diff --git a/src/features/threads-view/shell.dom.tsx b/src/features/threads-view/shell.dom.tsx index 49905c28..071a178d 100644 --- a/src/features/threads-view/shell.dom.tsx +++ b/src/features/threads-view/shell.dom.tsx @@ -226,9 +226,10 @@ function threadArchiveDisabled(row: ThreadsRowModel): boolean { function RenameRow({ row, actions, className }: { row: ThreadsRowModel; actions: ThreadsViewShellActions; className: string }): UiNode { const inputRef = useRef(null); + const renameBusy = row.rename.generating || row.rename.saving; useLayoutEffect(() => { - if (!row.rename.generating) focusThreadsRenameInput(inputRef.current); - }, [row.rename.draft, row.rename.generating]); + if (!renameBusy) focusThreadsRenameInput(inputRef.current); + }, [row.rename.draft, renameBusy]); return ( <> @@ -239,23 +240,23 @@ function RenameRow({ row, actions, className }: { row: ThreadsRowModel; actions: className="codex-panel-ui__nav-inline-input codex-panel-threads__rename-input" type="text" value={row.rename.draft} - disabled={row.rename.generating} + disabled={renameBusy} onInput={(event) => { actions.updateRename(row.threadId, event.currentTarget.value); }} onKeyDown={(event) => { if (event.key === "Enter") { event.preventDefault(); - if (!event.isComposing && !row.rename.generating) actions.saveRename(row.threadId, event.currentTarget.value); + if (!event.isComposing && !renameBusy) actions.saveRename(row.threadId, event.currentTarget.value); return; } if (event.key === "Escape") { event.preventDefault(); - actions.cancelRename(row.threadId); + if (!renameBusy) actions.cancelRename(row.threadId); } }} onBlur={(event) => { - if (!row.rename.generating) actions.saveRename(row.threadId, event.currentTarget.value); + if (!renameBusy) actions.saveRename(row.threadId, event.currentTarget.value); }} /> @@ -265,7 +266,7 @@ function RenameRow({ row, actions, className }: { row: ThreadsRowModel; actions: icon={row.rename.generating ? "x" : "sparkles"} label={row.rename.generating ? "Cancel auto-name" : "Auto-name thread"} className="codex-panel-threads__row-button" - disabled={!row.rename.generating && row.rename.autoNameDisabled} + disabled={row.rename.saving || (!row.rename.generating && row.rename.autoNameDisabled)} onPointerDown={(event) => { event.preventDefault(); event.stopPropagation(); diff --git a/src/features/threads/list/row-projection.ts b/src/features/threads/list/row-projection.ts index 2fc76e1e..ad71ed98 100644 --- a/src/features/threads/list/row-projection.ts +++ b/src/features/threads/list/row-projection.ts @@ -6,6 +6,7 @@ interface ThreadRowCoreRenameProjection { readonly active: boolean; readonly draft: string; readonly generating: boolean; + readonly saving: boolean; readonly autoNameDisabled: boolean; } @@ -38,6 +39,7 @@ export function threadRowCoreProjection(input: { active: rename !== undefined, draft: rename?.draft ?? threadRenameDraftTitle(input.thread), generating: rename?.kind === "generating", + saving: rename?.kind === "saving", autoNameDisabled: rename?.autoName.kind !== "ready", }, archiveConfirm: { diff --git a/tests/domain/threads/rename-lifecycle.test.ts b/tests/domain/threads/rename-lifecycle.test.ts index a148e3e7..bf0080f0 100644 --- a/tests/domain/threads/rename-lifecycle.test.ts +++ b/tests/domain/threads/rename-lifecycle.test.ts @@ -40,6 +40,25 @@ describe("thread rename lifecycle", () => { expect(transitionThreadRenameLifecycleState(idle, { type: "draft-updated", draft: "Stray" })).toBe(idle); }); + + it("serializes saving until the matching operation finishes", () => { + const editing = expectRenameState( + transitionThreadRenameLifecycleState(initialThreadRenameLifecycleState(), { type: "started", draft: "Draft" }), + ); + const saving = transitionThreadRenameLifecycleState(editing, { type: "save-started", saveToken: 1 }); + if (saving.kind !== "saving") throw new Error("Expected saving rename state."); + + expect(transitionThreadRenameLifecycleState(saving, { type: "draft-updated", draft: "Changed" })).toBe(saving); + expect(transitionThreadRenameLifecycleState(saving, { type: "cancelled" })).toBe(saving); + expect(transitionThreadRenameLifecycleState(saving, { type: "started", draft: "Replacement" })).toBe(saving); + expect(transitionThreadRenameLifecycleState(saving, { type: "save-succeeded", saveToken: 2 })).toBe(saving); + + const failed = transitionThreadRenameLifecycleState(saving, { type: "save-failed", saveToken: 1 }); + expect(failed).toEqual({ kind: "editing", draft: "Draft", autoName: { kind: "checking" } }); + + const savingAgain = transitionThreadRenameLifecycleState(failed, { type: "save-started", saveToken: 2 }); + expect(transitionThreadRenameLifecycleState(savingAgain, { type: "save-succeeded", saveToken: 2 })).toEqual({ kind: "idle" }); + }); }); type ThreadRenameGeneratingState = Extract; diff --git a/tests/features/chat/application/threads/rename-editor-actions.test.ts b/tests/features/chat/application/threads/rename-editor-actions.test.ts index f0530126..6d5b932a 100644 --- a/tests/features/chat/application/threads/rename-editor-actions.test.ts +++ b/tests/features/chat/application/threads/rename-editor-actions.test.ts @@ -149,11 +149,11 @@ describe("ThreadRenameEditorActions", () => { expect(actions.editState("thread")).toBeNull(); }); - it("does not save after cancellation while connection is pending", async () => { + it("blocks cancellation while a rename save is pending", async () => { const connection = deferred(); const renameThreadRequest = vi.fn().mockResolvedValue({}); const client = fakeClient({ renameThreadRequest }); - const { actions } = actionsFixture({ + const { actions, stateStore } = actionsFixture({ ensureConnected: vi.fn(() => connection.promise), currentClient: () => client, }); @@ -161,10 +161,12 @@ describe("ThreadRenameEditorActions", () => { actions.start("thread"); const save = actions.save("thread", "Saved title"); actions.cancel("thread"); + expect(actions.editState("thread")).toEqual({ draft: "Thread preview", generating: false }); + expect(stateStore.getState().ui.rename.kind).toBe("saving"); connection.resolve(undefined); await save; - expect(renameThreadRequest).not.toHaveBeenCalled(); + expect(renameThreadRequest).toHaveBeenCalledOnce(); expect(actions.editState("thread")).toBeNull(); }); @@ -205,7 +207,7 @@ describe("ThreadRenameEditorActions", () => { const readyContext = { userRequest: "Name this thread.", assistantResponse: "Done." }; const resolveThreadTitleContext = vi.fn().mockReturnValue(context.promise); const generateThreadTitle = vi.fn().mockResolvedValue("Generated title"); - const { actions } = actionsFixture({ + const { actions, stateStore } = actionsFixture({ currentClient: () => fakeClient({ renameThreadRequest: vi.fn(() => saved.promise) }), resolveThreadTitleContext, generateThreadTitle, @@ -219,6 +221,8 @@ describe("ThreadRenameEditorActions", () => { await flushPromises(); actions.updateDraft("thread", "Changed while saving"); context.resolve(readyContext); + await flushPromises(); + expect(stateStore.getState().ui.rename.kind).toBe("saving"); saved.reject(new Error("Rename failed.")); await saving; await flushPromises(); @@ -251,7 +255,7 @@ describe("ThreadRenameEditorActions", () => { expect(addSystemMessage).not.toHaveBeenCalled(); }); - it("does not clear a newer inline rename when an older save finishes", async () => { + it("blocks starting another inline rename while a save is pending", async () => { const saved = deferred(); const renameThreadRequest = vi.fn(() => saved.promise); const { actions, stateStore, notifyThreadRenamed } = actionsFixture({ @@ -259,19 +263,21 @@ describe("ThreadRenameEditorActions", () => { }); actions.start("thread"); + actions.updateDraft("thread", "Saved title"); const save = actions.save("thread", " Saved title "); await flushPromises(); actions.cancel("thread"); actions.start("thread"); actions.updateDraft("thread", "New draft"); + expect(actions.editState("thread")).toEqual({ draft: "Saved title", generating: false }); saved.resolve({}); await save; expect(renameThreadRequest).toHaveBeenCalledWith({ threadId: "thread", name: "Saved title" }); expect(stateStore.getState().threadList.listedThreads[0]?.name).toBe("Saved title"); expect(notifyThreadRenamed).toHaveBeenCalledWith("thread", "Saved title"); - expect(actions.editState("thread")).toEqual({ draft: "New draft", generating: false }); + expect(actions.editState("thread")).toBeNull(); }); it("ignores draft updates while auto-name generation is active", async () => { diff --git a/tests/features/chat/panel/surface/projections.test.ts b/tests/features/chat/panel/surface/projections.test.ts index 75e80c15..c1bb2aa2 100644 --- a/tests/features/chat/panel/surface/projections.test.ts +++ b/tests/features/chat/panel/surface/projections.test.ts @@ -81,6 +81,38 @@ describe("chat panel surface projections", () => { unmountUiRoot(parent); }); + it("disables other rename actions while a rename save is pending", () => { + let state = chatStateFixture({ + activeThread: { id: "thread" }, + threadList: { listedThreads: [threadFixture("thread", "Thread"), threadFixture("other", "Other")] }, + }); + state = chatStateWith(state, { + ui: { + toolbarPanel: "history", + rename: { + kind: "saving", + threadId: "thread", + draft: "Thread", + autoName: { kind: "unavailable" }, + saveToken: 1, + }, + }, + }); + const parent = renderWithShellModels(state, (models) => + h(ChatPanelToolbar, { + model: models.toolbar, + stateStore: createChatStateStore(state), + surface: toolbarSurfaceFixture(), + actions: toolbarActionsFixture(), + }), + ); + + const renameButtons = [...parent.querySelectorAll('[aria-label="Rename thread"]')]; + expect(renameButtons).toHaveLength(1); + expect(renameButtons[0]?.disabled).toBe(true); + unmountUiRoot(parent); + }); + it("disables subagent chat actions except starting a new chat", () => { let state = chatStateFixture(); state = chatStateWith(state, { diff --git a/tests/features/chat/ui/toolbar.test.ts b/tests/features/chat/ui/toolbar.test.ts index af6a0089..b4b89dcc 100644 --- a/tests/features/chat/ui/toolbar.test.ts +++ b/tests/features/chat/ui/toolbar.test.ts @@ -256,15 +256,23 @@ describe("Toolbar decisions", () => { historyOpen: true, openPanel: "history", threads: [ - { title: "Thread", threadId: "thread", selected: true, disabled: false, archiveDisabled: false, canArchive: true, rename: null }, + { + title: "Thread", + threadId: "thread", + selected: true, + renameDisabled: false, + archiveDisabled: false, + canArchive: true, + rename: null, + }, { title: "Editing", threadId: "editing", selected: false, - disabled: false, + renameDisabled: false, archiveDisabled: false, canArchive: true, - rename: { draft: "Draft title", generating: false, autoNameDisabled: false }, + rename: { draft: "Draft title", generating: false, saving: false, autoNameDisabled: false }, }, ], }), @@ -291,15 +299,23 @@ describe("Toolbar decisions", () => { historyOpen: true, openPanel: "history", threads: [ - { title: "Thread", threadId: "thread", selected: true, disabled: false, archiveDisabled: false, canArchive: true, rename: null }, + { + title: "Thread", + threadId: "thread", + selected: true, + renameDisabled: false, + archiveDisabled: false, + canArchive: true, + rename: null, + }, { title: "Editing", threadId: "editing", selected: false, - disabled: false, + renameDisabled: false, archiveDisabled: false, canArchive: true, - rename: { draft: "New title", generating: false, autoNameDisabled: false }, + rename: { draft: "New title", generating: false, saving: false, autoNameDisabled: false }, }, ], }), @@ -335,10 +351,10 @@ describe("Toolbar decisions", () => { title: "Editing", threadId: "editing", selected: false, - disabled: false, + renameDisabled: false, archiveDisabled: false, canArchive: true, - rename: { draft: "Draft title", generating: true, autoNameDisabled: false }, + rename: { draft: "Draft title", generating: true, saving: false, autoNameDisabled: false }, }, ], }), @@ -363,10 +379,10 @@ describe("Toolbar decisions", () => { title: "Editing", threadId: "editing", selected: false, - disabled: false, + renameDisabled: false, archiveDisabled: false, canArchive: true, - rename: { draft: "Draft title", generating: false, autoNameDisabled: false }, + rename: { draft: "Draft title", generating: false, saving: false, autoNameDisabled: false }, }, ], }), @@ -376,6 +392,44 @@ describe("Toolbar decisions", () => { parent.remove(); }); + it("locks rename controls while saving", () => { + const parent = document.createElement("div"); + const saveRenameThread = vi.fn(); + const cancelRenameThread = vi.fn(); + const autoNameThread = vi.fn(); + + mountToolbar( + parent, + toolbarModel({ + historyOpen: true, + openPanel: "history", + threads: [ + { + title: "Editing", + threadId: "editing", + selected: false, + renameDisabled: true, + archiveDisabled: false, + canArchive: true, + rename: { draft: "Draft title", generating: false, saving: true, autoNameDisabled: false }, + }, + ], + }), + toolbarActions({ saveRenameThread, cancelRenameThread, autoNameThread }), + ); + + const input = expectPresent(parent.querySelector(".codex-panel__thread-rename-input")); + expect(input.disabled).toBe(true); + input.dispatchEvent(new FocusEvent("blur")); + input.dispatchEvent(new KeyboardEvent("keydown", { key: "Escape", bubbles: true })); + const autoName = expectPresent(parent.querySelector('[aria-label="Auto-name thread"]')); + expect(autoName.disabled).toBe(true); + autoName.click(); + expect(saveRenameThread).not.toHaveBeenCalled(); + expect(cancelRenameThread).not.toHaveBeenCalled(); + expect(autoNameThread).not.toHaveBeenCalled(); + }); + it("disables auto-name while title context is unavailable", () => { const parent = document.createElement("div"); const autoNameThread = vi.fn(); @@ -390,10 +444,10 @@ describe("Toolbar decisions", () => { title: "Editing", threadId: "editing", selected: false, - disabled: false, + renameDisabled: false, archiveDisabled: false, canArchive: true, - rename: { draft: "Draft title", generating: false, autoNameDisabled: true }, + rename: { draft: "Draft title", generating: false, saving: false, autoNameDisabled: true }, }, ], }), @@ -421,7 +475,7 @@ describe("Toolbar decisions", () => { title: "Thread", threadId: "thread", selected: true, - disabled: false, + renameDisabled: false, archiveDisabled: false, canArchive: true, archiveConfirm: { active: true, defaultSaveMarkdown: true }, @@ -461,7 +515,7 @@ describe("Toolbar decisions", () => { title: "Thread", threadId: "thread", selected: true, - disabled: false, + renameDisabled: false, archiveDisabled: true, canArchive: true, archiveConfirm: { active: true, defaultSaveMarkdown: true }, @@ -558,7 +612,15 @@ function toolbarModel(overrides: Partial = {}): ToolbarViewMod debugDetails: () => "{}", openPanel: null, threads: [ - { title: "Thread", threadId: "thread", selected: true, disabled: false, archiveDisabled: false, canArchive: true, rename: null }, + { + title: "Thread", + threadId: "thread", + selected: true, + renameDisabled: false, + archiveDisabled: false, + canArchive: true, + rename: null, + }, ], connectLabel: "Reconnect", permissionsAndApprovals: [{ title: "Permissions", rows: [{ label: "Thread", value: "(none)" }] }], diff --git a/tests/features/threads-view/shell.test.tsx b/tests/features/threads-view/shell.test.tsx index 4b02734e..06c666cc 100644 --- a/tests/features/threads-view/shell.test.tsx +++ b/tests/features/threads-view/shell.test.tsx @@ -44,7 +44,7 @@ function rowFixture(overrides: Partial = {}): ThreadsRowModel { title, live: null, selected: false, - rename: { active: false, draft: title, generating: false, autoNameDisabled: true }, + rename: { active: false, draft: title, generating: false, saving: false, autoNameDisabled: true }, archiveConfirm: { active: false, defaultSaveMarkdown: false }, ...overrides, }; @@ -196,7 +196,7 @@ describe("threads view renderer decisions", () => { const actions = threadsViewActions(); const row = rowFixture({ title: "Old name", - rename: { active: true, draft: "Old name", generating: false, autoNameDisabled: false }, + rename: { active: true, draft: "Old name", generating: false, saving: false, autoNameDisabled: false }, }); renderThreadsViewShell(parent, { status: null, loading: false, rows: [row] }, actions); @@ -212,7 +212,7 @@ describe("threads view renderer decisions", () => { { status: null, loading: false, - rows: [{ ...row, rename: { active: true, draft: "New name", generating: false, autoNameDisabled: false } }], + rows: [{ ...row, rename: { active: true, draft: "New name", generating: false, saving: false, autoNameDisabled: false } }], }, actions, ); @@ -237,7 +237,7 @@ describe("threads view renderer decisions", () => { const actions = threadsViewActions(); const row = rowFixture({ title: "Old name", - rename: { active: true, draft: "Old name", generating: false, autoNameDisabled: false }, + rename: { active: true, draft: "Old name", generating: false, saving: false, autoNameDisabled: false }, }); renderThreadsViewShell(parent, { status: null, loading: false, rows: [row] }, actions); @@ -253,7 +253,7 @@ describe("threads view renderer decisions", () => { const actions = threadsViewActions(); const row = rowFixture({ title: "Old name", - rename: { active: true, draft: "Old name", generating: true, autoNameDisabled: false }, + rename: { active: true, draft: "Old name", generating: true, saving: false, autoNameDisabled: false }, }); renderThreadsViewShell(parent, { status: null, loading: false, rows: [row] }, actions); @@ -265,6 +265,28 @@ describe("threads view renderer decisions", () => { expect(actions.cancelAutoName).toHaveBeenCalledWith("thread"); }); + it("locks threads view rename controls while saving", () => { + const parent = document.createElement("div"); + const actions = threadsViewActions(); + const row = rowFixture({ + title: "Old name", + rename: { active: true, draft: "Old name", generating: false, saving: true, autoNameDisabled: false }, + }); + + renderThreadsViewShell(parent, { status: null, loading: false, rows: [row] }, actions); + + const input = expectPresent(parent.querySelector(".codex-panel-threads__rename-input")); + expect(input.disabled).toBe(true); + input.dispatchEvent(new FocusEvent("blur")); + input.dispatchEvent(new KeyboardEvent("keydown", { key: "Escape", bubbles: true })); + const autoName = expectPresent(parent.querySelector('[aria-label="Auto-name thread"]')); + expect(autoName.disabled).toBe(true); + autoName.click(); + expect(actions.saveRename).not.toHaveBeenCalled(); + expect(actions.cancelRename).not.toHaveBeenCalled(); + expect(actions.autoNameThread).not.toHaveBeenCalled(); + }); + it("disables history expansion during any shared thread fetch", () => { const parent = document.createElement("div"); const actions = threadsViewActions(); diff --git a/tests/features/threads-view/view.test.ts b/tests/features/threads-view/view.test.ts index ac4bd726..8f2937be 100644 --- a/tests/features/threads-view/view.test.ts +++ b/tests/features/threads-view/view.test.ts @@ -445,7 +445,7 @@ describe("CodexThreadsView", () => { }); }); - it("does not clear a newer rename edit when an older save finishes", async () => { + it("keeps the rename editor locked until a save finishes", async () => { const saved = deferred(); const renameThreadRequest = vi.fn(() => saved.promise); connectionMock.state.client = clientFixture({ @@ -471,10 +471,8 @@ describe("CodexThreadsView", () => { firstInput.dispatchEvent(new KeyboardEvent("keydown", { key: "Escape", bubbles: true })); view.containerEl.querySelector('[aria-label="Rename thread"]')?.click(); - const secondInput = view.containerEl.querySelector(".codex-panel-threads__rename-input"); - expect(secondInput).not.toBeNull(); - if (!secondInput) return; - changeInputValue(secondInput, "New draft"); + expect(view.containerEl.querySelector(".codex-panel-threads__rename-input")?.disabled).toBe(true); + expect(view.containerEl.querySelector('[aria-label="Rename thread"]')).toBeNull(); saved.resolve({}); @@ -484,11 +482,11 @@ describe("CodexThreadsView", () => { threadId: "thread", name: "Saved title", }); - expect(view.containerEl.querySelector(".codex-panel-threads__rename-input")?.value).toBe("New draft"); + expect(view.containerEl.querySelector(".codex-panel-threads__rename-input")).toBeNull(); }); }); - it("does not report an older save failure over a newer rename edit", async () => { + it("restores the same editor when a locked rename save fails", async () => { const saved = deferred(); const renameThreadRequest = vi.fn(() => saved.promise); connectionMock.state.client = clientFixture({ @@ -507,16 +505,14 @@ describe("CodexThreadsView", () => { firstInput.dispatchEvent(new KeyboardEvent("keydown", { key: "Escape", bubbles: true })); view.containerEl.querySelector('[aria-label="Rename thread"]')?.click(); - const secondInput = view.containerEl.querySelector(".codex-panel-threads__rename-input"); - if (!secondInput) throw new Error("Missing newer rename input"); - changeInputValue(secondInput, "New draft"); + expect(view.containerEl.querySelector(".codex-panel-threads__rename-input")?.disabled).toBe(true); - saved.reject(new Error("Older save failed.")); + saved.reject(new Error("Rename failed.")); for (let index = 0; index < 10; index += 1) await Promise.resolve(); - expect(view.containerEl.querySelector(".codex-panel-threads__rename-input")?.value).toBe("New draft"); - expect(view.containerEl.textContent).not.toContain("Older save failed."); - expect(notices).not.toContain("Older save failed."); + expect(view.containerEl.querySelector(".codex-panel-threads__rename-input")?.value).toBe("Saved title"); + expect(view.containerEl.querySelector(".codex-panel-threads__rename-input")?.disabled).toBe(false); + expect(notices).toContain("Rename failed."); }); it("notifies a current rename failure without adding list status", async () => { diff --git a/tests/features/threads/list/row-projection.test.ts b/tests/features/threads/list/row-projection.test.ts index 31c0c1eb..1f0cc162 100644 --- a/tests/features/threads/list/row-projection.test.ts +++ b/tests/features/threads/list/row-projection.test.ts @@ -32,6 +32,7 @@ describe("thread row core projection", () => { active: false, draft: "Preview title", generating: false, + saving: false, autoNameDisabled: true, }); expect(threadRowCoreProjection({ thread: thread({ name: null, preview: "" }), selected: false }).rename.draft).toBe("");