diff --git a/biome.jsonc b/biome.jsonc index c6989044..2cbf643d 100644 --- a/biome.jsonc +++ b/biome.jsonc @@ -56,7 +56,13 @@ }, { "path": "./scripts/grit/import-boundaries/no-app-server-connection-boundary-imports.grit", - "includes": ["**/src/app-server/protocol/**/*.ts", "**/src/domain/**/*.ts", "**/src/shared/**/*.{ts,tsx}"] + "includes": [ + "**/src/app-server/protocol/**/*.ts", + "**/src/domain/**/*.ts", + "**/src/shared/**/*.{ts,tsx}", + "**/src/features/selection-rewrite/**/*.{ts,tsx}", + "!**/src/features/selection-rewrite/app-server-transport.ts" + ] }, { "path": "./scripts/grit/import-boundaries/no-app-server-protocol-boundary-imports.grit", diff --git a/src/features/selection-rewrite/runner.ts b/src/features/selection-rewrite/app-server-transport.ts similarity index 62% rename from src/features/selection-rewrite/runner.ts rename to src/features/selection-rewrite/app-server-transport.ts index 65668a4b..f4705ce0 100644 --- a/src/features/selection-rewrite/runner.ts +++ b/src/features/selection-rewrite/app-server-transport.ts @@ -6,71 +6,63 @@ import { type StructuredTurnOutputSchema, } from "../../app-server/services/ephemeral-structured-turn"; import { resolvedRuntimeOverrideForClient } from "../../app-server/services/runtime-overrides"; -import type { SelectionRewriteRuntimeSettings } from "./model"; -import { type SelectionRewriteOutput, SelectionRewriteOutputError, selectionRewriteOutputParseResultFromText } from "./output"; +import { SelectionRewriteOutputError, selectionRewriteOutputParseResultFromText } from "./output"; import { SELECTION_REWRITE_DEVELOPER_INSTRUCTIONS, SELECTION_REWRITE_SERVICE_NAME } from "./prompt"; +import type { SelectionRewriteTransport, SelectionRewriteTransportRequest } from "./transport"; const SELECTION_REWRITE_TIMEOUT_MS = 120_000; const SELECTION_REWRITE_OUTPUT_SCHEMA: StructuredTurnOutputSchema = { type: "object", - properties: { - replacementText: { - type: "string", - }, - }, + properties: { replacementText: { type: "string" } }, required: ["replacementText"], additionalProperties: false, }; -export interface RunSelectionRewriteOptions { - codexPath: string; - cwd: string; - prompt: string; - runtimeSettings?: SelectionRewriteRuntimeSettings; - onActivity?: (activity: SelectionRewriteActivity) => void; - onPreview?: (text: string) => void; - signal?: AbortSignal; - clientFactory?: SelectionRewriteClientFactory; -} - -export type SelectionRewriteActivity = "reasoning" | "writing"; - type SelectionRewriteClient = EphemeralStructuredTurnClient & ModelMetadataClient; type SelectionRewriteClientFactory = (codexPath: string, cwd: string, handlers: AppServerClientHandlers) => SelectionRewriteClient; -export async function runSelectionRewrite(options: RunSelectionRewriteOptions): Promise { +export interface AppServerSelectionRewriteTransportOptions { + codexPath(): string; + cwd: string; + clientFactory?: SelectionRewriteClientFactory; +} + +export function createAppServerSelectionRewriteTransport(options: AppServerSelectionRewriteTransportOptions): SelectionRewriteTransport { + return { + generate: (request) => runAppServerSelectionRewrite(options, request), + }; +} + +async function runAppServerSelectionRewrite(options: AppServerSelectionRewriteTransportOptions, request: SelectionRewriteTransportRequest) { let preview = ""; - const runtimeSettings = options.runtimeSettings; const lastAgentText = await runEphemeralStructuredTurnForLastAgentText({ - codexPath: options.codexPath, + codexPath: options.codexPath(), cwd: options.cwd, serviceName: SELECTION_REWRITE_SERVICE_NAME, developerInstructions: SELECTION_REWRITE_DEVELOPER_INSTRUCTIONS, - prompt: options.prompt, + prompt: request.prompt, outputSchema: SELECTION_REWRITE_OUTPUT_SCHEMA, timeoutMs: SELECTION_REWRITE_TIMEOUT_MS, serverRequests: { kind: "reject", message: "Selection rewrite does not handle server requests." }, exitedMessage: "Selection rewrite app-server exited.", timedOutMessage: "Timed out while rewriting the selection.", abortMessage: "Selection rewrite cancelled.", - signal: options.signal, - resolveRuntime: runtimeSettings - ? (client) => - resolvedRuntimeOverrideForClient(client, { - model: runtimeSettings.rewriteSelectionModel, - effort: runtimeSettings.rewriteSelectionEffort, - }) - : undefined, + signal: request.signal, + resolveRuntime: (client) => + resolvedRuntimeOverrideForClient(client, { + model: request.runtimeSettings.rewriteSelectionModel, + effort: request.runtimeSettings.rewriteSelectionEffort, + }), clientFactory: options.clientFactory, onProgress: (event) => { if (event.type === "reasoning-activity") { - options.onActivity?.("reasoning"); + request.onActivity("reasoning"); return; } preview = `${preview}${event.delta}`; - options.onActivity?.("writing"); - options.onPreview?.(preview); + request.onActivity("writing"); + request.onPreview(preview); }, }); const { output, rawText } = selectionRewriteOutputParseResultFromText(lastAgentText); diff --git a/src/features/selection-rewrite/command.obsidian.ts b/src/features/selection-rewrite/command.obsidian.ts index e497fe06..014aa0bf 100644 --- a/src/features/selection-rewrite/command.obsidian.ts +++ b/src/features/selection-rewrite/command.obsidian.ts @@ -2,16 +2,13 @@ import { type Editor, MarkdownView, Notice, type Plugin } from "obsidian"; import type { SendShortcut } from "../../domain/input/send-shortcut"; import type { SelectionRewriteRuntimeSettings, SelectionRewriteState } from "./model"; import { SelectionRewritePopover } from "./popover.dom"; +import type { SelectionRewriteTransport } from "./transport"; export interface SelectionRewriteCommandHost extends Plugin { - settings: { - codexPath: string; - sendShortcut: SendShortcut; - } & SelectionRewriteRuntimeSettings; - vaultPath: string; + settings: { sendShortcut: SendShortcut } & SelectionRewriteRuntimeSettings; } -export function registerSelectionRewriteCommand(plugin: SelectionRewriteCommandHost): void { +export function registerSelectionRewriteCommand(plugin: SelectionRewriteCommandHost, transport: SelectionRewriteTransport): void { const activePopovers = new Set(); plugin.register(() => { @@ -57,8 +54,6 @@ export function registerSelectionRewriteCommand(plugin: SelectionRewriteCommandH let popover: SelectionRewritePopover | null = null; popover = new SelectionRewritePopover({ - codexPath: plugin.settings.codexPath, - cwd: plugin.vaultPath, editor, onClose: () => { if (popover) activePopovers.delete(popover); @@ -66,6 +61,7 @@ export function registerSelectionRewriteCommand(plugin: SelectionRewriteCommandH runtimeSettings: plugin.settings, sendShortcut: plugin.settings.sendShortcut, state: rewriteState, + transport, viewDocument, viewWindow, }); diff --git a/src/features/selection-rewrite/popover.dom.tsx b/src/features/selection-rewrite/popover.dom.tsx index c1c06d83..a18c002b 100644 --- a/src/features/selection-rewrite/popover.dom.tsx +++ b/src/features/selection-rewrite/popover.dom.tsx @@ -17,6 +17,7 @@ import { import { positionSelectionRewritePopover } from "./position.dom"; import { SelectionRewriteSession, type SelectionRewriteSessionStatus } from "./session"; +import type { SelectionRewriteTransport } from "./transport"; const POPOVER_MARGIN = 8; @@ -33,13 +34,12 @@ function isSelectionRewriteActionKey(event: { } export interface SelectionRewritePopoverOptions { - codexPath: string; - cwd: string; editor: Editor; onClose?: () => void; runtimeSettings: SelectionRewriteRuntimeSettings; sendShortcut: SendShortcut; state: SelectionRewriteState; + transport: SelectionRewriteTransport; viewDocument: Document; viewWindow: Window; } diff --git a/src/features/selection-rewrite/session.ts b/src/features/selection-rewrite/session.ts index 2438da97..a6b3313e 100644 --- a/src/features/selection-rewrite/session.ts +++ b/src/features/selection-rewrite/session.ts @@ -7,7 +7,7 @@ import { } from "./model"; import { SelectionRewriteOutputError } from "./output"; import { buildSelectionRewritePrompt } from "./prompt"; -import { runSelectionRewrite, type SelectionRewriteActivity } from "./runner"; +import type { SelectionRewriteActivity, SelectionRewriteTransport } from "./transport"; const MAX_SELECTION_REWRITE_INSTRUCTION_HISTORY = 20; @@ -17,10 +17,9 @@ type SelectionRewriteGenerationRunState = { kind: "idle" } | { kind: "running"; type ActiveSelectionRewriteGenerationRun = Extract; export interface SelectionRewriteSessionOptions { - codexPath: string; - cwd: string; runtimeSettings: SelectionRewriteRuntimeSettings; state: SelectionRewriteState; + transport: SelectionRewriteTransport; } export interface SelectionRewriteSessionStatus { @@ -90,9 +89,7 @@ export class SelectionRewriteSession { hooks.render(); try { - const output = await runSelectionRewrite({ - codexPath: this.options.codexPath, - cwd: this.options.cwd, + const output = await this.options.transport.generate({ prompt: buildSelectionRewritePrompt(this.state), runtimeSettings: this.options.runtimeSettings, onActivity: (activity) => { diff --git a/src/features/selection-rewrite/transport.ts b/src/features/selection-rewrite/transport.ts new file mode 100644 index 00000000..dc653f85 --- /dev/null +++ b/src/features/selection-rewrite/transport.ts @@ -0,0 +1,16 @@ +import type { SelectionRewriteRuntimeSettings } from "./model"; +import type { SelectionRewriteOutput } from "./output"; + +export type SelectionRewriteActivity = "reasoning" | "writing"; + +export interface SelectionRewriteTransportRequest { + prompt: string; + runtimeSettings: SelectionRewriteRuntimeSettings; + onActivity(activity: SelectionRewriteActivity): void; + onPreview(text: string): void; + signal: AbortSignal; +} + +export interface SelectionRewriteTransport { + generate(request: SelectionRewriteTransportRequest): Promise; +} diff --git a/src/main.ts b/src/main.ts index 00d150f2..cbef91f5 100644 --- a/src/main.ts +++ b/src/main.ts @@ -2,6 +2,7 @@ import { Notice, Plugin } from "obsidian"; import { VIEW_TYPE_CODEX_PANEL, VIEW_TYPE_CODEX_THREADS, VIEW_TYPE_CODEX_TURN_DIFF } from "./constants"; import { CodexChatView } from "./features/chat/host/view.obsidian"; +import { createAppServerSelectionRewriteTransport } from "./features/selection-rewrite/app-server-transport"; import { registerSelectionRewriteCommand } from "./features/selection-rewrite/command.obsidian"; import { CodexThreadsView } from "./features/threads-view/view.obsidian"; import { CodexTurnDiffView } from "./features/turn-diff/view.obsidian"; @@ -70,7 +71,13 @@ export default class CodexPanelPlugin extends Plugin { callback: () => void this.runtime.startNewChat().catch(reportCommandError), }); - registerSelectionRewriteCommand(this); + registerSelectionRewriteCommand( + this, + createAppServerSelectionRewriteTransport({ + codexPath: () => this.settings.codexPath, + cwd: this.vaultPath, + }), + ); this.addSettingTab(new CodexPanelSettingTab(this.app, this, this.runtime.settingTabHost())); diff --git a/tests/features/selection-rewrite/selection-rewrite-command.test.ts b/tests/features/selection-rewrite/selection-rewrite-command.test.ts index 101f61d2..722f8562 100644 --- a/tests/features/selection-rewrite/selection-rewrite-command.test.ts +++ b/tests/features/selection-rewrite/selection-rewrite-command.test.ts @@ -5,6 +5,7 @@ import { describe, expect, it, vi } from "vitest"; import { registerSelectionRewriteCommand } from "../../../src/features/selection-rewrite/command.obsidian"; import type { SelectionRewritePopoverOptions } from "../../../src/features/selection-rewrite/popover.dom"; +import type { SelectionRewriteTransport } from "../../../src/features/selection-rewrite/transport"; const popoverMock = vi.hoisted(() => { const instances: { options: SelectionRewritePopoverOptions; open: ReturnType; close: ReturnType }[] = []; @@ -60,7 +61,8 @@ describe("selection rewrite command", () => { Object.assign(view, { containerEl: { doc: document } }); view.file = Object.assign(new TFile(), { path: "Draft.md", basename: "Draft" }); - registerSelectionRewriteCommand(plugin as never); + const transport: SelectionRewriteTransport = { generate: vi.fn() }; + registerSelectionRewriteCommand(plugin as never, transport); expect(addedCommand.current).not.toBeNull(); addedCommand.current?.editorCallback(editor, view); from.line = 99; @@ -77,6 +79,7 @@ describe("selection rewrite command", () => { }, }); expect(popoverMock.instances[0]?.open).toHaveBeenCalledOnce(); + expect(popoverMock.instances[0]?.options.transport).toBe(transport); 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 80bdf490..33563570 100644 --- a/tests/features/selection-rewrite/selection-rewrite.test.ts +++ b/tests/features/selection-rewrite/selection-rewrite.test.ts @@ -8,6 +8,10 @@ import type { TurnItem, TurnRecord } from "../../../src/app-server/protocol/turn import type { AppServerStartStructuredTurnOptions } from "../../../src/app-server/services/turns"; import type { ModelMetadata, ReasoningEffort } from "../../../src/domain/catalog/metadata"; import type { ServerInitialization } from "../../../src/domain/server/initialization"; +import { + type AppServerSelectionRewriteTransportOptions, + createAppServerSelectionRewriteTransport, +} from "../../../src/features/selection-rewrite/app-server-transport"; import { buildSelectionUnifiedDiff } from "../../../src/features/selection-rewrite/diff"; import { canApplySelectionRewrite, @@ -19,8 +23,7 @@ import { selectionRewriteOutputParseResultFromText } from "../../../src/features import { SelectionRewritePopover } from "../../../src/features/selection-rewrite/popover.dom"; import { positionSelectionRewritePopover } from "../../../src/features/selection-rewrite/position.dom"; import { buildSelectionRewritePrompt } from "../../../src/features/selection-rewrite/prompt"; -import * as selectionRewriteRunner from "../../../src/features/selection-rewrite/runner"; -import { runSelectionRewrite } from "../../../src/features/selection-rewrite/runner"; +import type { SelectionRewriteTransportRequest } from "../../../src/features/selection-rewrite/transport"; import type { ModelListResponse } from "../../../src/generated/app-server/v2/ModelListResponse"; import type { ThreadStartResponse } from "../../../src/generated/app-server/v2/ThreadStartResponse"; import { deferred } from "../../support/async"; @@ -31,14 +34,18 @@ type Turn = TurnRecord; interface TurnStartResponse { turn: TurnRecord; } -type SelectionRewriteClientFactory = NonNullable[0]["clientFactory"]>; +type SelectionRewriteClientFactory = NonNullable; type SelectionRewriteClient = ReturnType; +type SelectionRewriteTestRunOptions = SelectionRewriteTransportRequest & { clientFactory: SelectionRewriteClientFactory }; + +const selectionRewriteGenerate = vi.fn(); installObsidianDomShims(); (globalThis as typeof globalThis & { IS_REACT_ACT_ENVIRONMENT?: boolean }).IS_REACT_ACT_ENVIRONMENT = true; beforeEach(() => { document.body.replaceChildren(); + selectionRewriteGenerate.mockReset(); }); afterEach(() => { @@ -376,7 +383,7 @@ describe("selection rewrite popover", () => { it("generates from the Enter shortcut, renders a preview diff, and applies from the action shortcut", async () => { const editor = editorFixture(); const onClose = vi.fn(); - vi.spyOn(selectionRewriteRunner, "runSelectionRewrite").mockImplementation(async (options) => { + selectionRewriteGenerate.mockImplementation(async (options) => { options.onPreview?.("Rewritten sentence."); return { replacementText: "Rewritten sentence." }; }); @@ -397,7 +404,7 @@ describe("selection rewrite popover", () => { await Promise.resolve(); }); - expect(selectionRewriteRunner.runSelectionRewrite).toHaveBeenCalledOnce(); + expect(selectionRewriteGenerate).toHaveBeenCalledOnce(); expect(document.querySelector(".codex-panel-selection-rewrite__diff")?.textContent).toContain("Rewritten sentence."); const apply = expectPresent(document.querySelector('button[aria-label="Apply"]')); expect(apply.disabled).toBe(false); @@ -473,7 +480,7 @@ describe("selection rewrite popover", () => { it("aborts the active generation when closed and ignores its late result", async () => { const rewrite = deferred<{ replacementText: string }>(); let signal: AbortSignal | undefined; - vi.spyOn(selectionRewriteRunner, "runSelectionRewrite").mockImplementation((options) => { + selectionRewriteGenerate.mockImplementation((options) => { signal = options.signal; return rewrite.promise; }); @@ -501,7 +508,7 @@ describe("selection rewrite popover", () => { const rewrite = deferred<{ replacementText: string }>(); const options = popoverOptions(); const preview: { current: ((text: string) => void) | null } = { current: null }; - vi.spyOn(selectionRewriteRunner, "runSelectionRewrite").mockImplementation((runOptions) => { + selectionRewriteGenerate.mockImplementation((runOptions) => { preview.current = runOptions.onPreview ?? null; return rewrite.promise; }); @@ -528,7 +535,7 @@ describe("selection rewrite popover", () => { 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); + selectionRewriteGenerate.mockReturnValue(rewrite.promise); const popover = new SelectionRewritePopover(popoverOptions()); openPopover(popover); @@ -539,7 +546,7 @@ describe("selection rewrite popover", () => { await Promise.resolve(); }); - expect(selectionRewriteRunner.runSelectionRewrite).toHaveBeenCalledOnce(); + expect(selectionRewriteGenerate).toHaveBeenCalledOnce(); rewrite.resolve({ replacementText: "Rewritten once." }); await act(async () => { @@ -550,7 +557,7 @@ describe("selection rewrite popover", () => { }); it("restores the current draft after accidental instruction history navigation", async () => { - vi.spyOn(selectionRewriteRunner, "runSelectionRewrite").mockResolvedValue({ replacementText: "Rewritten." }); + selectionRewriteGenerate.mockResolvedValue({ replacementText: "Rewritten." }); await generateInstructionHistoryItem("History item one."); await generateInstructionHistoryItem("History item two."); @@ -576,7 +583,7 @@ describe("selection rewrite popover", () => { }); it("preserves edited instruction history drafts while navigating history", async () => { - vi.spyOn(selectionRewriteRunner, "runSelectionRewrite").mockResolvedValue({ replacementText: "Rewritten." }); + selectionRewriteGenerate.mockResolvedValue({ replacementText: "Rewritten." }); await generateInstructionHistoryItem("History item one."); await generateInstructionHistoryItem("History item two."); @@ -617,7 +624,7 @@ describe("selection rewrite popover", () => { }); it("records edited history items as new entries only when generated", async () => { - vi.spyOn(selectionRewriteRunner, "runSelectionRewrite").mockResolvedValue({ replacementText: "Rewritten." }); + selectionRewriteGenerate.mockResolvedValue({ replacementText: "Rewritten." }); await generateInstructionHistoryItem("Rewrite as bullets."); @@ -739,12 +746,13 @@ function popoverOptions( overrides: Partial[0]> = {}, ): ConstructorParameters[0] { return { - codexPath: "/usr/local/bin/codex", - cwd: "/vault", editor: editorFixture().editor, runtimeSettings: { rewriteSelectionModel: null, rewriteSelectionEffort: null }, sendShortcut: "enter", state: rewriteState(), + transport: { + generate: (request) => selectionRewriteGenerate(request), + }, viewDocument: document, viewWindow: window, ...overrides, @@ -791,11 +799,22 @@ async function flushPromises(): Promise { await Promise.resolve(); } -function runOptions(clientFactory: SelectionRewriteClientFactory): Parameters[0] { - return { - codexPath: "/bin/codex", +function runSelectionRewrite(options: SelectionRewriteTestRunOptions) { + const { clientFactory, ...request } = options; + return createAppServerSelectionRewriteTransport({ + codexPath: () => "/bin/codex", cwd: "/vault", + clientFactory, + }).generate(request); +} + +function runOptions(clientFactory: SelectionRewriteClientFactory): SelectionRewriteTestRunOptions { + return { prompt: "Rewrite this.", + runtimeSettings: { rewriteSelectionModel: null, rewriteSelectionEffort: null }, + onActivity: () => undefined, + onPreview: () => undefined, + signal: new AbortController().signal, clientFactory, }; } diff --git a/tests/scripts/grit-policy.test.mjs b/tests/scripts/grit-policy.test.mjs index 3b7a56b9..3d8ed16a 100644 --- a/tests/scripts/grit-policy.test.mjs +++ b/tests/scripts/grit-policy.test.mjs @@ -941,6 +941,10 @@ describe("Biome Grit include boundaries", () => { expect(pluginMessages(report, "src/shared/runtime/connection-client.ts")).toEqual([ "Do not import app-server connection internals from this module. Keep connection usage at app-server adapters.", ]); + expect(pluginMessages(report, "src/features/selection-rewrite/session.ts")).toEqual([ + "Do not import app-server connection internals from this module. Keep connection usage at app-server adapters.", + ]); + expect(pluginDiagnostics(report, "src/features/selection-rewrite/app-server-transport.ts")).toEqual([]); }); it("keeps app-server root modules from becoming boundary escape hatches", async () => { @@ -1430,6 +1434,22 @@ export type Client = AppServerClient; ` import type { AppServerClient } from "src/app-server/connection/client"; +export type Client = AppServerClient; +`.trimStart(), + ); + await writeFile( + path.join(cwd, "src/features/selection-rewrite/session.ts"), + ` +import type { AppServerClient } from "../../app-server/connection/client"; + +export type Client = AppServerClient; +`.trimStart(), + ); + await writeFile( + path.join(cwd, "src/features/selection-rewrite/app-server-transport.ts"), + ` +import type { AppServerClient } from "../../app-server/connection/client"; + export type Client = AppServerClient; `.trimStart(), ); @@ -1512,6 +1532,8 @@ export async function read(client: AppServerClient): Promise { "src/app-server/services/catalog.ts", "src/domain/connection-client.ts", "src/shared/runtime/connection-client.ts", + "src/features/selection-rewrite/session.ts", + "src/features/selection-rewrite/app-server-transport.ts", "src/features/chat/application/threads/history.ts", "src/app-server/services/threads.ts", "src/features/chat/host/bundles/connection-bundle.ts",