From 75bf1a0fb8c62944d45c8396ac618945fabebf50 Mon Sep 17 00:00:00 2001 From: murashit Date: Fri, 10 Jul 2026 12:15:22 +0900 Subject: [PATCH] Cancel asynchronous UI work after owner disposal --- .../chat/panel/composer-controller.ts | 10 ++++ src/features/threads-view/session.ts | 22 ++++++-- src/settings/dynamic-sections-controller.ts | 24 +++++++-- src/shared/runtime/owner-lifetime.ts | 28 +++++++++++ .../chat/panel/composer-controller.test.ts | 50 +++++++++++++++++++ tests/features/threads-view/view.test.ts | 31 ++++++++++++ tests/settings/settings-tab.test.ts | 21 ++++++++ 7 files changed, 179 insertions(+), 7 deletions(-) create mode 100644 src/shared/runtime/owner-lifetime.ts diff --git a/src/features/chat/panel/composer-controller.ts b/src/features/chat/panel/composer-controller.ts index dd4a84db..3a25db28 100644 --- a/src/features/chat/panel/composer-controller.ts +++ b/src/features/chat/panel/composer-controller.ts @@ -1,4 +1,5 @@ import { isComposerSendKey, type SendShortcut } from "../../../domain/input/send-shortcut"; +import { OwnerLifetime } from "../../../shared/runtime/owner-lifetime"; import { type ComposerAttachment, type ComposerAttachmentHandler, @@ -72,6 +73,7 @@ export interface ChatComposerRenderActions { } export class ChatComposerController { + private readonly lifetime = new OwnerLifetime(); private composer: HTMLTextAreaElement | null = null; private attachments: ComposerAttachment[] = []; private pendingAttachmentTransfers = new Set>(); @@ -145,6 +147,7 @@ export class ChatComposerController { } dispose(): void { + this.lifetime.dispose(); this.composer = null; this.options.noteCandidateProvider.dispose(); this.options.contextReferenceProvider.dispose(); @@ -350,12 +353,16 @@ export class ChatComposerController { } private async saveTransferredFiles(handler: ComposerAttachmentHandler, files: readonly File[]): Promise { + const lifetime = this.lifetime.signal(); + if (!this.lifetime.isCurrent(lifetime)) return; try { const attachments = await handler.saveFiles(files); + if (!this.lifetime.isCurrent(lifetime)) return; if (attachments.length === 0) return; this.attachments = [...this.attachments, ...attachments]; this.insertAttachmentMarkers(attachments); } catch (error) { + if (!this.lifetime.isCurrent(lifetime)) return; this.options.onAttachmentError?.(error instanceof Error ? error.message : String(error)); } } @@ -456,11 +463,14 @@ export class ChatComposerController { private async submitAfterAttachmentTransfers(actions: ChatComposerRenderActions): Promise { if (this.submitAfterAttachmentTransfersActive) return; + const lifetime = this.lifetime.signal(); + if (!this.lifetime.isCurrent(lifetime)) return; this.submitAfterAttachmentTransfersActive = true; try { while (this.pendingAttachmentTransfers.size > 0) { await Promise.allSettled([...this.pendingAttachmentTransfers]); } + if (!this.lifetime.isCurrent(lifetime)) return; actions.submit(); } finally { this.submitAfterAttachmentTransfersActive = false; diff --git a/src/features/threads-view/session.ts b/src/features/threads-view/session.ts index a609f1fb..99bab7c6 100644 --- a/src/features/threads-view/session.ts +++ b/src/features/threads-view/session.ts @@ -7,6 +7,7 @@ import { isStaleAppServerSharedQueryContextError } from "../../app-server/query/ import type { ReasoningEffort } from "../../domain/catalog/metadata"; import type { ArchiveExportSettings } from "../../domain/threads/archive-markdown"; import type { Thread } from "../../domain/threads/model"; +import { OwnerLifetime } from "../../shared/runtime/owner-lifetime"; import type { ThreadCatalogActiveReader, ThreadCatalogEventSink } from "../threads/catalog/thread-catalog"; import type { ArchiveExportDestination } from "../threads/workflows/archive-export"; import { createThreadOperations, type ThreadOperations } from "../threads/workflows/thread-operations"; @@ -66,6 +67,7 @@ type ThreadsViewStatus = | { kind: "error"; message: string }; export class ThreadsViewSession { + private readonly lifetime = new OwnerLifetime(); private readonly operations: ThreadOperations; private readonly titleService: ThreadTitleService; private readonly deferredTasks: ThreadsViewDeferredTasks; @@ -104,6 +106,7 @@ export class ThreadsViewSession { } open(): void { + this.lifetime.activate(); this.environment.registerPointerDown((event) => { this.cancelArchiveConfirmOnOutsidePointer(event); }); @@ -120,6 +123,7 @@ export class ThreadsViewSession { } close(): void { + this.lifetime.dispose(); this.refreshLifecycle = transitionThreadsViewRefreshLifecycle(this.refreshLifecycle, { type: "invalidated" }); this.deferredTasks.clearAll(); this.unsubscribeThreads?.(); @@ -128,6 +132,8 @@ export class ThreadsViewSession { } async refresh(): Promise { + const lifetime = this.lifetime.signal(); + if (!this.lifetime.isCurrent(lifetime)) return; const refresh = this.startRefresh(); if (!this.currentThreadsSnapshot()) { this.status = { kind: "loading", message: "Loading threads..." }; @@ -135,11 +141,12 @@ export class ThreadsViewSession { this.render(); try { const threads = await this.host.threadCatalog.refreshActive(); - if (this.isStaleRefresh(refresh)) return; + if (!this.lifetime.isCurrent(lifetime) || this.isStaleRefresh(refresh)) return; this.threads = threads; this.threadsLoaded = true; this.status = threads.length === 0 ? { kind: "empty", message: "No threads" } : { kind: "idle" }; } catch (error) { + if (!this.lifetime.isCurrent(lifetime)) return; if (isStaleAppServerSharedQueryContextError(error)) return; if (!this.currentThreadsSnapshot()) { this.status = { kind: "error", message: error instanceof Error ? error.message : String(error) }; @@ -204,6 +211,7 @@ export class ThreadsViewSession { } private render(): void { + if (!this.lifetime.isActive()) return; renderThreadsViewShell( this.environment.root, { @@ -272,12 +280,13 @@ export class ThreadsViewSession { } private async saveRename(threadId: string, value: string): Promise { + const lifetime = this.lifetime.signal(); const editingState = this.renameStates.get(threadId); if (!editingState || editingState.kind === "generating") return; try { if (this.renameStates.get(threadId) !== editingState) return; const result = await this.operations.renameThread(threadId, value); - if (this.renameStates.get(threadId) !== editingState) return; + if (!this.lifetime.isCurrent(lifetime) || this.renameStates.get(threadId) !== editingState) return; if (!result) { this.cancelRename(threadId); return; @@ -285,12 +294,14 @@ export class ThreadsViewSession { this.renameStates.delete(threadId); this.render(); } catch (error) { + if (!this.lifetime.isCurrent(lifetime)) return; this.status = { kind: "error", message: error instanceof Error ? error.message : String(error) }; this.render(); } } private async autoNameThread(threadId: string): Promise { + const lifetime = this.lifetime.signal(); const previousState = this.renameStates.get(threadId); const generatingState = this.transitionRenameState(threadId, { type: "auto-name-started", @@ -303,13 +314,15 @@ export class ThreadsViewSession { try { if (this.renameStates.get(threadId) !== generatingState) return; const title = await this.titleService.generateTitle(threadId); + if (!this.lifetime.isCurrent(lifetime)) return; this.transitionRenameState(threadId, { type: "auto-name-generated", generatingState, title }); } catch (error) { + if (!this.lifetime.isCurrent(lifetime)) return; if (this.renameStates.get(threadId) === generatingState) { this.status = { kind: "error", message: error instanceof Error ? error.message : String(error) }; } } finally { - this.finishAutoNameThread(threadId, generatingState); + if (this.lifetime.isCurrent(lifetime)) this.finishAutoNameThread(threadId, generatingState); } } @@ -326,14 +339,17 @@ export class ThreadsViewSession { } private async archiveThread(threadId: string, saveMarkdown: boolean): Promise { + const lifetime = this.lifetime.signal(); try { await this.operations.archiveThread(threadId, { saveMarkdown, }); + if (!this.lifetime.isCurrent(lifetime)) return; this.host.closeOpenPanelsForThread(threadId); if (this.archiveConfirmThreadId === threadId) this.archiveConfirmThreadId = null; this.renameStates.delete(threadId); } catch (error) { + if (!this.lifetime.isCurrent(lifetime)) return; this.status = { kind: "error", message: error instanceof Error ? error.message : String(error) }; this.render(); } diff --git a/src/settings/dynamic-sections-controller.ts b/src/settings/dynamic-sections-controller.ts index be51609e..ff34be46 100644 --- a/src/settings/dynamic-sections-controller.ts +++ b/src/settings/dynamic-sections-controller.ts @@ -3,6 +3,7 @@ import type { HookItem, ModelMetadata, ReasoningEffort } from "../domain/catalog import { findModelMetadataByIdOrName, sortedModelMetadata, supportedEffortsForModelMetadata } from "../domain/catalog/metadata"; import type { Thread } from "../domain/threads/model"; import { threadArchiveDisplayTitle } from "../domain/threads/title"; +import { OwnerLifetime } from "../shared/runtime/owner-lifetime"; import { isStaleSettingsDynamicDataContextError } from "./dynamic-data"; import type { SettingsDynamicSectionsHost } from "./host"; @@ -31,6 +32,7 @@ export interface SettingsDynamicSectionsSnapshot { } export class SettingsDynamicSectionsController { + private readonly lifetime = new OwnerLifetime(); private dynamicSectionsAutoLoadStarted = false; private modelsOperationToken = 0; private hooksOperationToken = 0; @@ -56,6 +58,7 @@ export class SettingsDynamicSectionsController { activate(): void { if (this.unsubscribeModels) return; + this.lifetime.activate(); this.models = [...(this.host.dynamicData.modelsSnapshot() ?? [])]; const archivedThreads = this.host.dynamicData.archivedThreadsSnapshot(); if (archivedThreads) { @@ -101,6 +104,10 @@ export class SettingsDynamicSectionsController { } dispose(): void { + this.lifetime.dispose(); + this.modelsOperationToken += 1; + this.hooksOperationToken += 1; + this.archivedThreadsOperationToken += 1; this.unsubscribeModels?.(); this.unsubscribeModels = null; this.unsubscribeArchivedThreads?.(); @@ -126,6 +133,8 @@ export class SettingsDynamicSectionsController { } async refreshDynamicSections(options: { forceModels?: boolean } = {}): Promise { + const lifetime = this.lifetime.signal(); + if (!this.lifetime.isCurrent(lifetime)) return; this.dynamicSectionsAutoLoadStarted = true; const modelsOperationToken = this.nextModelsOperationToken(); const hooksOperationToken = this.nextHooksOperationToken(); @@ -142,6 +151,7 @@ export class SettingsDynamicSectionsController { this.host.dynamicData.loadHooks(), this.host.dynamicData.refreshArchivedThreads(), ] as const); + if (!this.lifetime.isCurrent(lifetime)) return; if (this.isStaleModelsOperation(modelsOperationToken)) { // A newer models operation owns this section. @@ -185,6 +195,7 @@ export class SettingsDynamicSectionsController { ); } } catch (error) { + if (!this.lifetime.isCurrent(lifetime)) return; failedCount = 3; const message = errorMessage(error); if (!this.isStaleModelsOperation(modelsOperationToken)) { @@ -197,10 +208,12 @@ export class SettingsDynamicSectionsController { this.archivedThreadsLifecycle = settingsDynamicSectionFailed(`Could not load archived threads: ${message}`); } } finally { - if (failedCount > 0) { - this.callbacks.notify("Could not refresh all Codex details."); + if (this.lifetime.isCurrent(lifetime)) { + if (failedCount > 0) { + this.callbacks.notify("Could not refresh all Codex details."); + } + this.callbacks.display(); } - this.callbacks.display(); } } @@ -333,9 +346,12 @@ export class SettingsDynamicSectionsController { failureNotice: string; operation: (operationToken: number) => Promise; }): Promise { + const lifetime = this.lifetime.signal(); + if (!this.lifetime.isCurrent(lifetime)) return; const operationToken = options.section === "hooks" ? this.nextHooksOperationToken() : this.nextArchivedThreadsOperationToken(); const stale = (): boolean => - options.section === "hooks" ? this.isStaleHooksOperation(operationToken) : this.isStaleArchivedThreadsOperation(operationToken); + !this.lifetime.isCurrent(lifetime) || + (options.section === "hooks" ? this.isStaleHooksOperation(operationToken) : this.isStaleArchivedThreadsOperation(operationToken)); const setLifecycle = (state: SettingsDynamicSectionLifecycleState): void => { if (options.section === "hooks") { this.hooksLifecycle = state; diff --git a/src/shared/runtime/owner-lifetime.ts b/src/shared/runtime/owner-lifetime.ts new file mode 100644 index 00000000..f1a5e253 --- /dev/null +++ b/src/shared/runtime/owner-lifetime.ts @@ -0,0 +1,28 @@ +export class OwnerLifetime { + private controller = new AbortController(); + private active = true; + + signal(): AbortSignal { + return this.controller.signal; + } + + isCurrent(signal: AbortSignal): boolean { + return this.active && !signal.aborted && signal === this.controller.signal; + } + + isActive(): boolean { + return this.active; + } + + activate(): void { + if (this.active) return; + this.controller = new AbortController(); + this.active = true; + } + + dispose(): void { + if (!this.active) return; + this.active = false; + this.controller.abort(); + } +} diff --git a/tests/features/chat/panel/composer-controller.test.ts b/tests/features/chat/panel/composer-controller.test.ts index fcad0425..ecc16f08 100644 --- a/tests/features/chat/panel/composer-controller.test.ts +++ b/tests/features/chat/panel/composer-controller.test.ts @@ -15,6 +15,7 @@ import { ChatComposerController, type ChatComposerRenderActions } from "../../.. import type { ChatPanelComposerReadModel } from "../../../../src/features/chat/panel/shell-read-model"; import { ComposerShell } from "../../../../src/features/chat/ui/composer"; import { renderUiRoot, unmountUiRoot } from "../../../../src/shared/dom/preact-root.dom"; +import { deferred } from "../../../support/async"; import { installObsidianDomShims } from "../../../support/dom"; import { composerReadModelFromChatState } from "../support/shell-read-model"; @@ -447,6 +448,55 @@ describe("ChatComposerController", () => { expect(submit).toHaveBeenCalledOnce(); }); + it("does not submit or apply saved attachments after disposal", async () => { + const stateStore = createChatStateStore(); + const parent = document.createElement("div"); + const saved = deferred(); + const attachmentHandler: ComposerAttachmentHandler = { + saveFiles: vi.fn(() => saved.promise), + }; + const submit = vi.fn(); + const controller = new ChatComposerController({ + noteCandidateProvider: noteProvider(), + contextReferenceProvider: contextProvider(), + attachmentHandler, + sourcePath: () => "", + stateStore, + viewId: "view", + referenceActiveNoteOnSend: () => false, + sendShortcut: () => "enter", + scrollThreadFromComposerEdges: () => false, + threadScrollFromComposer: vi.fn(), + canInterrupt: (_state) => false, + composerProjection: defaultComposerProjection, + currentModelForSuggestions: () => null, + togglePlan: vi.fn(), + toggleAutoReview: vi.fn(), + toggleFast: vi.fn(), + onDraftChange: vi.fn(), + onHeightChange: vi.fn(), + }); + + renderComposerController(parent, controller, stateStore, { submit }); + composer(parent).dispatchEvent(transferEvent("paste", "clipboardData", [new File(["image"], "diagram.png", { type: "image/png" })])); + composer(parent).dispatchEvent(new KeyboardEvent("keydown", { bubbles: true, cancelable: true, key: "Enter" })); + controller.dispose(); + saved.resolve([ + { + kind: "image", + name: "diagram", + path: "Codex Attachments/diagram.png", + marker: "![[Codex Attachments/diagram.png]]", + }, + ]); + await flushComposerAttachment(); + await flushComposerAttachment(); + + expect(submit).not.toHaveBeenCalled(); + expect(stateStore.getState().composer.draft).toBe(""); + expect(controller.captureInputSnapshot().attachments).toEqual([]); + }); + it("saves dropped non-image files, inserts a wikilink, and sends a file mention", async () => { const stateStore = createChatStateStore(); const parent = document.createElement("div"); diff --git a/tests/features/threads-view/view.test.ts b/tests/features/threads-view/view.test.ts index 2e7e9214..b46d9176 100644 --- a/tests/features/threads-view/view.test.ts +++ b/tests/features/threads-view/view.test.ts @@ -468,6 +468,37 @@ describe("CodexThreadsView", () => { }); }); + it("does not remount the threads view when auto-name finishes after close", async () => { + const generatedTitle = deferred(); + namingMock.generateThreadTitleWithCodex.mockReturnValue(generatedTitle.promise); + connectionMock.state.client = clientFixture({ + "thread/list": vi.fn().mockResolvedValue({ data: [threadFixture({ id: "thread", preview: "Thread preview" })] }), + "thread/turns/list": vi.fn().mockResolvedValue({ + data: [ + turnFixture([ + { type: "userMessage", id: "u1", clientId: null, content: [{ type: "text", text: "name this", text_elements: [] }] }, + { type: "agentMessage", id: "a1", text: "done", phase: "final_answer", memoryCitation: null }, + ]), + ], + nextCursor: null, + }), + }); + const view = await threadsView(); + + await view.refresh(); + view.containerEl.querySelector('[aria-label="Rename thread"]')?.click(); + view.containerEl.querySelector('[aria-label="Auto-name thread"]')?.click(); + await waitForAsyncWork(() => { + expect(namingMock.generateThreadTitleWithCodex).toHaveBeenCalledOnce(); + }); + await view.onClose(); + generatedTitle.resolve("Late title"); + for (let index = 0; index < 10; index += 1) await Promise.resolve(); + + expect(view.containerEl.childElementCount).toBe(0); + expect(view.containerEl.textContent).not.toContain("Late title"); + }); + it("keeps a manually edited rename draft when threads view auto-name finishes later", async () => { const threadTurnsList = vi.fn().mockResolvedValue({ data: [ diff --git a/tests/settings/settings-tab.test.ts b/tests/settings/settings-tab.test.ts index aaca60b8..bcddfac9 100644 --- a/tests/settings/settings-tab.test.ts +++ b/tests/settings/settings-tab.test.ts @@ -424,6 +424,27 @@ describe("settings tab", () => { expect(controller.snapshot().archivedThreads.map((thread) => thread.preview)).toEqual(["New"]); }); + it("does not display dynamic refresh results after disposal", async () => { + const models = deferred(); + const display = vi.fn(); + const controller = new SettingsDynamicSectionsController( + settingsTabHost({ + refreshModels: vi.fn(() => models.promise), + refreshArchived: vi.fn().mockResolvedValue([]), + }), + { display, notify: vi.fn() }, + ); + + const refresh = controller.refreshDynamicSections(); + await flushPromises(); + display.mockClear(); + controller.dispose(); + models.resolve([]); + await refresh; + + expect(display).not.toHaveBeenCalled(); + }); + it("ignores stale hook reload results after a newer dynamic operation completes", async () => { const staleHooks = deferred<{ data: { cwd: string; hooks: CatalogHookMetadata[]; warnings: string[]; errors: unknown[] }[];