fix(threads): scope mutations to their operation context

This commit is contained in:
murashit 2026-07-17 06:07:30 +09:00
parent f7efd475a4
commit cb6e89b1ae
4 changed files with 210 additions and 29 deletions

View file

@ -70,16 +70,21 @@ export function createThreadRenameEditorActions(host: ThreadRenameEditorActionsH
if (current.kind === "idle" || current.threadId !== threadId || current.kind === "generating") return;
const editingState = current;
await host.ensureConnected();
if (renameState(host) !== editingState) return;
try {
await host.ensureConnected();
if (renameState(host) !== editingState) return;
const result = await host.renameThread(threadId, value);
if (!result) {
if (renameState(host) === editingState) action.cancel(threadId);
return;
}
if (renameState(host) === editingState) {
dispatch(host, { type: "ui/rename-cleared" });
const result = await host.renameThread(threadId, value);
if (!result) {
if (renameState(host) === editingState) action.cancel(threadId);
return;
}
if (renameState(host) === editingState) {
dispatch(host, { type: "ui/rename-cleared" });
}
} catch (error) {
if (renameState(host) !== editingState) return;
host.addSystemMessage(error instanceof Error ? error.message : String(error));
}
},

View file

@ -57,6 +57,11 @@ type ThreadsViewStatus =
| { kind: "log"; message: string }
| { kind: "error"; message: string };
interface ThreadsViewOperationLease {
readonly lifetime: AbortSignal;
readonly appServerContextGeneration: number;
}
export class ThreadsViewSession {
private readonly lifetime = new OwnerLifetime();
private readonly operations: ThreadOperations;
@ -71,7 +76,7 @@ export class ThreadsViewSession {
private unsubscribeThreads: (() => void) | null = null;
private archiveConfirmThreadId: string | null = null;
private observedAppServerContext: AppServerQueryContext;
private operationGeneration = 0;
private appServerContextGeneration = 0;
constructor(private readonly environment: ThreadsViewSessionEnvironment) {
this.observedAppServerContext = this.currentAppServerContext();
@ -172,7 +177,7 @@ export class ThreadsViewSession {
}
prepareAppServerContextChange(): void {
this.operationGeneration += 1;
this.appServerContextGeneration += 1;
this.titleService.invalidate();
this.activeRefresh = null;
this.renderTask.clear();
@ -321,19 +326,16 @@ export class ThreadsViewSession {
}
private async saveRename(threadId: string, value: string): Promise<void> {
const lifetime = this.lifetime.signal();
const operationGeneration = this.operationGeneration;
const lease = this.captureOperationLease();
const editingState = this.renameStates.get(threadId);
if (!editingState || editingState.kind === "generating") return;
try {
if (this.renameStates.get(threadId) !== editingState) return;
const operationIsCurrent = () => operationGeneration === this.operationGeneration;
const result = await this.operations.renameThread(threadId, value, {
shouldStart: operationIsCurrent,
shouldPublish: operationIsCurrent,
shouldStart: () => this.operationContextIsCurrent(lease),
shouldPublish: () => this.operationContextIsCurrent(lease),
});
if (!operationIsCurrent()) return;
if (!this.lifetime.isCurrent(lifetime) || this.renameStates.get(threadId) !== editingState) return;
if (!this.operationViewIsCurrent(lease) || this.renameStates.get(threadId) !== editingState) return;
if (!result) {
this.cancelRename(threadId);
return;
@ -341,15 +343,14 @@ export class ThreadsViewSession {
this.renameStates.delete(threadId);
this.render();
} catch (error) {
if (!this.lifetime.isCurrent(lifetime)) return;
if (!this.operationViewIsCurrent(lease) || this.renameStates.get(threadId) !== editingState) return;
this.status = { kind: "error", message: error instanceof Error ? error.message : String(error) };
this.render();
}
}
private async autoNameThread(threadId: string): Promise<void> {
const lifetime = this.lifetime.signal();
const operationGeneration = this.operationGeneration;
const lease = this.captureOperationLease();
const previousState = this.renameStates.get(threadId);
const generationToken = this.nextRenameGenerationToken;
const generatingState = this.transitionRenameState(threadId, {
@ -363,15 +364,15 @@ export class ThreadsViewSession {
try {
if (this.renameStates.get(threadId) !== generatingState) return;
const title = await this.titleService.generateTitle(threadId);
if (!this.lifetime.isCurrent(lifetime) || operationGeneration !== this.operationGeneration) return;
if (!this.operationViewIsCurrent(lease)) return;
this.transitionRenameState(threadId, { type: "generation-succeeded", generationToken, draft: title });
} catch (error) {
if (!this.lifetime.isCurrent(lifetime) || operationGeneration !== this.operationGeneration) return;
if (!this.operationViewIsCurrent(lease)) return;
if (this.renameStates.get(threadId) === generatingState) {
this.status = { kind: "error", message: error instanceof Error ? error.message : String(error) };
}
} finally {
if (this.lifetime.isCurrent(lifetime) && operationGeneration === this.operationGeneration) {
if (this.operationViewIsCurrent(lease)) {
this.finishAutoNameThread(threadId, generationToken);
}
}
@ -390,24 +391,38 @@ export class ThreadsViewSession {
}
private async archiveThread(threadId: string, saveMarkdown: boolean): Promise<void> {
const lifetime = this.lifetime.signal();
const operationGeneration = this.operationGeneration;
const lease = this.captureOperationLease();
try {
await this.operations.archiveThread(threadId, {
saveMarkdown,
shouldPublish: () => operationGeneration === this.operationGeneration,
shouldPublish: () => this.operationContextIsCurrent(lease),
});
if (!this.lifetime.isCurrent(lifetime) || operationGeneration !== this.operationGeneration) return;
if (!this.operationViewIsCurrent(lease)) return;
this.host.closeOpenPanelsForThread(threadId);
if (this.archiveConfirmThreadId === threadId) this.archiveConfirmThreadId = null;
this.renameStates.delete(threadId);
} catch (error) {
if (!this.lifetime.isCurrent(lifetime)) return;
if (!this.operationViewIsCurrent(lease)) return;
this.status = { kind: "error", message: error instanceof Error ? error.message : String(error) };
this.render();
}
}
private captureOperationLease(): ThreadsViewOperationLease {
return {
lifetime: this.lifetime.signal(),
appServerContextGeneration: this.appServerContextGeneration,
};
}
private operationContextIsCurrent(lease: ThreadsViewOperationLease): boolean {
return lease.appServerContextGeneration === this.appServerContextGeneration;
}
private operationViewIsCurrent(lease: ThreadsViewOperationLease): boolean {
return this.operationContextIsCurrent(lease) && this.lifetime.isCurrent(lease.lifetime);
}
private finishAutoNameThread(threadId: string, generationToken: number): void {
const previousState = this.renameStates.get(threadId);
const nextState = this.transitionRenameState(threadId, { type: "generation-finished", generationToken });

View file

@ -87,6 +87,59 @@ describe("ThreadRenameEditorActions", () => {
expect(actions.editState("thread")).toBeNull();
});
it("keeps the draft and reports a connection failure while saving", async () => {
const addSystemMessage = vi.fn();
const { actions } = actionsFixture({
ensureConnected: vi.fn().mockRejectedValue(new Error("Could not connect.")),
addSystemMessage,
});
actions.start("thread");
actions.updateDraft("thread", "Unsaved draft");
await actions.save("thread", "Unsaved draft");
expect(actions.editState("thread")).toEqual({ draft: "Unsaved draft", generating: false });
expect(addSystemMessage).toHaveBeenCalledWith("Could not connect.");
});
it("keeps the draft and reports an app-server rename failure", async () => {
const addSystemMessage = vi.fn();
const renameThreadRequest = vi.fn().mockRejectedValue(new Error("Rename failed."));
const { actions } = actionsFixture({
currentClient: () => fakeClient({ renameThreadRequest }),
addSystemMessage,
});
actions.start("thread");
actions.updateDraft("thread", "Unsaved draft");
await actions.save("thread", "Unsaved draft");
expect(actions.editState("thread")).toEqual({ draft: "Unsaved draft", generating: false });
expect(addSystemMessage).toHaveBeenCalledWith("Rename failed.");
});
it("does not report a delayed save failure after the edit is invalidated", async () => {
const saved = deferred<object>();
const addSystemMessage = vi.fn();
const { actions } = actionsFixture({
currentClient: () => fakeClient({ renameThreadRequest: vi.fn(() => saved.promise) }),
addSystemMessage,
});
actions.start("thread");
const save = actions.save("thread", "Old draft");
await flushPromises();
actions.invalidate();
actions.start("thread");
actions.updateDraft("thread", "New draft");
saved.reject(new Error("Stale rename failed."));
await save;
expect(actions.editState("thread")).toEqual({ draft: "New draft", generating: false });
expect(addSystemMessage).not.toHaveBeenCalled();
});
it("does not clear a newer inline rename when an older save finishes", async () => {
const saved = deferred<object>();
const renameThreadRequest = vi.fn(() => saved.promise);

View file

@ -422,6 +422,36 @@ describe("CodexThreadsView", () => {
});
});
it("does not report an older save failure over a newer rename edit", async () => {
const saved = deferred<object>();
const renameThreadRequest = vi.fn(() => saved.promise);
connectionMock.state.client = clientFixture({
"thread/list": vi.fn().mockResolvedValue({ data: [threadFixture({ id: "thread", preview: "Thread preview" })] }),
"thread/name/set": renameThreadRequest,
});
const view = await threadsView(threadsHost());
await view.refresh();
view.containerEl.querySelector<HTMLButtonElement>('[aria-label="Rename thread"]')?.click();
const firstInput = view.containerEl.querySelector<HTMLInputElement>(".codex-panel-threads__rename-input");
if (!firstInput) throw new Error("Missing rename input");
changeInputValue(firstInput, "Saved title");
firstInput.dispatchEvent(new FocusEvent("blur"));
await waitForAsyncWork(() => expect(renameThreadRequest).toHaveBeenCalledOnce());
firstInput.dispatchEvent(new KeyboardEvent("keydown", { key: "Escape", bubbles: true }));
view.containerEl.querySelector<HTMLButtonElement>('[aria-label="Rename thread"]')?.click();
const secondInput = view.containerEl.querySelector<HTMLInputElement>(".codex-panel-threads__rename-input");
if (!secondInput) throw new Error("Missing newer rename input");
changeInputValue(secondInput, "New draft");
saved.reject(new Error("Older save failed."));
for (let index = 0; index < 10; index += 1) await Promise.resolve();
expect(view.containerEl.querySelector<HTMLInputElement>(".codex-panel-threads__rename-input")?.value).toBe("New draft");
expect(view.containerEl.textContent).not.toContain("Older save failed.");
});
it("clears old rows and suppresses a delayed rename across an app-server context replacement", async () => {
let codexPath = "codex-a";
const saved = deferred<object>();
@ -468,6 +498,70 @@ describe("CodexThreadsView", () => {
expect(view.containerEl.textContent).not.toContain("Old thread");
});
it("does not report a delayed rename rejection after an app-server context replacement", async () => {
let codexPath = "codex-a";
const saved = deferred<object>();
const renameThreadRequest = vi.fn(() => saved.promise);
connectionMock.state.client = clientFixture({
"thread/list": vi.fn().mockResolvedValue({ data: [threadFixture({ id: "thread-a", preview: "Old thread" })] }),
"thread/name/set": renameThreadRequest,
});
const host = threadsHost({ settings: contextSettings(() => codexPath) });
const view = await threadsView(host);
await view.onOpen();
await waitForAsyncWork(() => expect(view.containerEl.textContent).toContain("Old thread"));
view.containerEl.querySelector<HTMLButtonElement>('[aria-label="Rename thread"]')?.click();
const input = view.containerEl.querySelector<HTMLInputElement>(".codex-panel-threads__rename-input");
if (!input) throw new Error("Missing rename input");
changeInputValue(input, "Renamed old thread");
input.dispatchEvent(new FocusEvent("blur"));
await waitForAsyncWork(() => expect(renameThreadRequest).toHaveBeenCalledOnce());
view.prepareAppServerContextChange();
codexPath = "codex-b";
connectionMock.state.client = clientFixture({ "thread/list": vi.fn().mockResolvedValue({ data: [] }) });
view.refreshSettings();
await waitForAsyncWork(() => expect(view.containerEl.textContent).toContain("No threads"));
saved.reject(new Error("Old rename failed."));
for (let index = 0; index < 10; index += 1) await Promise.resolve();
expect(view.containerEl.textContent).toContain("No threads");
expect(view.containerEl.textContent).not.toContain("Old rename failed.");
});
it("does not report a delayed archive rejection after an app-server context replacement", async () => {
let codexPath = "codex-a";
const archived = deferred<object>();
const archiveThreadRequest = vi.fn(() => archived.promise);
connectionMock.state.client = clientFixture({
"thread/list": vi.fn().mockResolvedValue({ data: [threadFixture({ id: "thread-a", preview: "Old thread" })] }),
"thread/archive": archiveThreadRequest,
});
const closeOpenPanelsForThread = vi.fn();
const host = threadsHost({
settings: contextSettings(() => codexPath),
closeOpenPanelsForThread,
});
const view = await threadsView(host);
await view.onOpen();
await waitForAsyncWork(() => expect(view.containerEl.textContent).toContain("Old thread"));
view.containerEl.querySelector<HTMLButtonElement>('[aria-label="Archive thread"]')?.click();
view.containerEl.querySelector<HTMLButtonElement>('[aria-label="Archive thread without saving"]')?.click();
await waitForAsyncWork(() => expect(archiveThreadRequest).toHaveBeenCalledOnce());
view.prepareAppServerContextChange();
codexPath = "codex-b";
connectionMock.state.client = clientFixture({ "thread/list": vi.fn().mockResolvedValue({ data: [] }) });
view.refreshSettings();
await waitForAsyncWork(() => expect(view.containerEl.textContent).toContain("No threads"));
archived.reject(new Error("Old archive failed."));
for (let index = 0; index < 10; index += 1) await Promise.resolve();
expect(view.containerEl.textContent).toContain("No threads");
expect(view.containerEl.textContent).not.toContain("Old archive failed.");
expect(closeOpenPanelsForThread).not.toHaveBeenCalled();
});
it("auto-names a thread rename draft from completed history", async () => {
const threadTurnsList = vi.fn().mockResolvedValue({
data: [
@ -702,6 +796,20 @@ function threadsHost(overrides: Record<string, unknown> = {}) {
};
}
function contextSettings(codexPath: () => string) {
return {
archiveExportEnabled: () => DEFAULT_SETTINGS.archiveExportEnabled,
codexPath,
threadNamingModel: () => DEFAULT_SETTINGS.threadNamingModel,
threadNamingEffort: () => DEFAULT_SETTINGS.threadNamingEffort,
archiveExportSettings: () => ({
archiveExportFolderTemplate: DEFAULT_SETTINGS.archiveExportFolderTemplate,
archiveExportFilenameTemplate: DEFAULT_SETTINGS.archiveExportFilenameTemplate,
archiveExportTags: DEFAULT_SETTINGS.archiveExportTags,
}),
};
}
async function threadsView(host = threadsHost()) {
const { CodexThreadsView } = await import("../../../src/features/threads-view/view.obsidian");
const containerEl = document.createElement("div");