diff --git a/src/features/selection-rewrite/popover.tsx b/src/features/selection-rewrite/popover.tsx index d63c9a7f..918fb1fa 100644 --- a/src/features/selection-rewrite/popover.tsx +++ b/src/features/selection-rewrite/popover.tsx @@ -137,9 +137,11 @@ export class SelectionRewritePopover { prompt: buildSelectionRewritePrompt(this.options.state), runtimeSettings: this.options.runtimeSettings, onActivity: (activity) => { + if (generationRun.abortController.signal.aborted || !this.isActiveGenerationRun(generationRun)) return; this.updateActivity(activity); }, onPreview: (text) => { + if (generationRun.abortController.signal.aborted || !this.isActiveGenerationRun(generationRun)) return; this.updatePreview(text); }, signal: generationRun.abortController.signal, diff --git a/tests/features/selection-rewrite/selection-rewrite-command.test.ts b/tests/features/selection-rewrite/selection-rewrite-command.test.ts new file mode 100644 index 00000000..0a86edbc --- /dev/null +++ b/tests/features/selection-rewrite/selection-rewrite-command.test.ts @@ -0,0 +1,83 @@ +// @vitest-environment jsdom + +import { describe, expect, it, vi } from "vitest"; +import { MarkdownView, TFile } from "obsidian"; + +import { registerSelectionRewriteCommand } from "../../../src/features/selection-rewrite/command"; +import type { SelectionRewritePopoverOptions } from "../../../src/features/selection-rewrite/popover"; + +const popoverMock = vi.hoisted(() => { + const instances: { options: SelectionRewritePopoverOptions; open: ReturnType; close: ReturnType }[] = []; + return { + instances, + reset(): void { + instances.length = 0; + }, + }; +}); + +vi.mock("../../../src/features/selection-rewrite/popover", () => { + class SelectionRewritePopover { + readonly open = vi.fn(); + readonly close = vi.fn(); + + constructor(readonly options: SelectionRewritePopoverOptions) { + popoverMock.instances.push({ options, open: this.open, close: this.close }); + } + } + + return { SelectionRewritePopover }; +}); + +describe("selection rewrite command", () => { + it("captures the unsaved editor buffer and clones the target range", () => { + popoverMock.reset(); + const addedCommand = { current: null as null | { editorCallback: (editor: unknown, view: unknown) => void } }; + const cleanup = { current: null as null | (() => void) }; + const plugin = { + settings: { + codexPath: "/usr/local/bin/codex", + sendShortcut: "enter", + rewriteSelectionModel: null, + rewriteSelectionEffort: null, + }, + vaultPath: "/vault", + register: vi.fn((callback: () => void) => { + cleanup.current = callback; + }), + addCommand: vi.fn((command: { editorCallback: (editor: unknown, view: unknown) => void }) => { + addedCommand.current = command; + }), + }; + const from = { line: 1, ch: 0 }; + const to = { line: 1, ch: 22 }; + const editor = { + getSelection: vi.fn(() => "Revise this sentence."), + getValue: vi.fn(() => "# Draft heading\n\nRevise this sentence.\n\nUnsaved paragraph."), + getCursor: vi.fn((which: "from" | "to") => (which === "from" ? from : to)), + }; + const view = new MarkdownView({} as never); + view.file = Object.assign(new TFile(), { path: "Draft.md", basename: "Draft" }); + + registerSelectionRewriteCommand(plugin as never); + expect(addedCommand.current).not.toBeNull(); + addedCommand.current?.editorCallback(editor, view); + from.line = 99; + to.ch = 99; + + const captured = popoverMock.instances[0]?.options.state; + expect(captured).toMatchObject({ + filePath: "Draft.md", + originalText: "Revise this sentence.", + noteText: "# Draft heading\n\nRevise this sentence.\n\nUnsaved paragraph.", + targetRange: { + from: { line: 1, ch: 0 }, + to: { line: 1, ch: 22 }, + }, + }); + expect(popoverMock.instances[0]?.open).toHaveBeenCalledOnce(); + + cleanup.current?.(); + expect(popoverMock.instances[0]?.close).toHaveBeenCalledOnce(); + }); +}); diff --git a/tests/features/selection-rewrite/selection-rewrite.test.ts b/tests/features/selection-rewrite/selection-rewrite.test.ts index 09e90ae3..f349074d 100644 --- a/tests/features/selection-rewrite/selection-rewrite.test.ts +++ b/tests/features/selection-rewrite/selection-rewrite.test.ts @@ -283,6 +283,29 @@ describe("selection rewrite popover", () => { expect(onClose).toHaveBeenCalledOnce(); }); + it("refuses to apply when the current editor range no longer matches the original selection", async () => { + const editor = editorFixture({ currentText: "Changed sentence." }); + const popover = new SelectionRewritePopover( + popoverOptions({ + editor: editor.editor, + state: rewriteState({ status: "preview", replacementText: "Rewritten sentence." }), + }), + ); + + openPopover(popover); + await act(async () => { + expectPresent(document.querySelector('button[aria-label="Apply"]')).click(); + await Promise.resolve(); + }); + + expect(editor.getRange).toHaveBeenCalledWith({ line: 1, ch: 0 }, { line: 1, ch: 22 }); + expect(editor.replaceRange).not.toHaveBeenCalled(); + expect(document.querySelector(".codex-panel-selection-rewrite")).not.toBeNull(); + expect(document.body.textContent).toContain("Selection changed. Generate the rewrite again before applying."); + + closePopover(popover); + }); + it("unmounts and removes the Preact popover when closed", () => { const onClose = vi.fn(); const popover = new SelectionRewritePopover(popoverOptions({ onClose })); @@ -345,6 +368,35 @@ describe("selection rewrite popover", () => { expect(document.body.textContent).not.toContain("Late rewrite"); }); + it("ignores late generation preview callbacks after close", async () => { + const rewrite = deferred<{ replacementText: string }>(); + const options = popoverOptions(); + const preview: { current: ((text: string) => void) | null } = { current: null }; + vi.spyOn(selectionRewriteRunner, "runSelectionRewrite").mockImplementation((runOptions) => { + preview.current = runOptions.onPreview ?? null; + return rewrite.promise; + }); + const popover = new SelectionRewritePopover(options); + + openPopover(popover); + await act(async () => { + expectPresent(document.querySelector('button[aria-label="Generate"]')).click(); + await Promise.resolve(); + }); + + closePopover(popover); + const latePreview = preview.current; + if (!latePreview) throw new Error("Expected preview callback to be captured"); + latePreview("Late partial"); + + expect(options.state.streamText).toBe(""); + + rewrite.resolve({ replacementText: "Late rewrite" }); + await act(async () => { + await flushPromises(); + }); + }); + it("keeps only one generation run active while a rewrite is pending", async () => { const rewrite = deferred<{ replacementText: string }>(); vi.spyOn(selectionRewriteRunner, "runSelectionRewrite").mockReturnValue(rewrite.promise); @@ -579,18 +631,21 @@ function popoverOptions( }; } -function editorFixture(): { +function editorFixture(options: { currentText?: string } = {}): { editor: ConstructorParameters[0]["editor"]; + getRange: ReturnType; replaceRange: ReturnType; } { + const getRange = vi.fn(() => options.currentText ?? "Revise this sentence."); const replaceRange = vi.fn(); return { editor: { getCursor: () => ({ line: 1, ch: 22 }), - getRange: vi.fn(() => "Revise this sentence."), + getRange, posToOffset: () => 0, replaceRange, } as unknown as ConstructorParameters[0]["editor"], + getRange, replaceRange, }; }