Move selection rewrite behind a feature-owned transport

This commit is contained in:
murashit 2026-07-12 20:16:41 +09:00
parent 8cd3bafaff
commit 9d2b750d77
10 changed files with 129 additions and 71 deletions

View file

@ -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",

View file

@ -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<SelectionRewriteOutput> {
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);

View file

@ -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<SelectionRewritePopover>();
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,
});

View file

@ -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;
}

View file

@ -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<SelectionRewriteGenerationRunState, { kind: "running" }>;
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) => {

View file

@ -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<SelectionRewriteOutput>;
}

View file

@ -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()));

View file

@ -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<typeof vi.fn>; close: ReturnType<typeof vi.fn> }[] = [];
@ -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();

View file

@ -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<Parameters<typeof runSelectionRewrite>[0]["clientFactory"]>;
type SelectionRewriteClientFactory = NonNullable<AppServerSelectionRewriteTransportOptions["clientFactory"]>;
type SelectionRewriteClient = ReturnType<SelectionRewriteClientFactory>;
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<HTMLButtonElement>('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<ConstructorParameters<typeof SelectionRewritePopover>[0]> = {},
): ConstructorParameters<typeof SelectionRewritePopover>[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<void> {
await Promise.resolve();
}
function runOptions(clientFactory: SelectionRewriteClientFactory): Parameters<typeof runSelectionRewrite>[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,
};
}

View file

@ -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<void> {
"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",