diff --git a/eslint.config.mjs b/eslint.config.mjs index 1d8ba941..72828019 100644 --- a/eslint.config.mjs +++ b/eslint.config.mjs @@ -79,6 +79,7 @@ const chatImperativeDomBridgeFiles = [ "src/features/chat/ui/turn-diff.tsx", ]; const nonChatImperativeDomBridgeFiles = [ + "src/app-server/structured-ephemeral-turn.ts", "src/features/selection-rewrite/popover.tsx", "src/features/selection-rewrite/runner.ts", "src/features/thread-picker/modal.ts", diff --git a/src/app-server/model-options.ts b/src/app-server/model-options.ts new file mode 100644 index 00000000..69ea9978 --- /dev/null +++ b/src/app-server/model-options.ts @@ -0,0 +1,23 @@ +import { AppServerClient } from "./client"; +import { panelModelOptionsFromAppServerModels } from "./catalog-model"; +import type { PanelModelOption } from "../domain/catalog/metadata"; + +export async function loadPanelModelOptions(codexPath: string, cwd: string, includeHidden = false): Promise { + let client!: AppServerClient; + client = new AppServerClient(codexPath, cwd, { + onNotification: () => undefined, + onServerRequest: (request) => { + client.rejectServerRequest(request.id, -32601, "Model option loading does not handle server requests."); + }, + onLog: () => undefined, + onExit: () => undefined, + }); + + try { + await client.connect(); + const response = await client.listModels(includeHidden); + return panelModelOptionsFromAppServerModels(response.data); + } finally { + client.disconnect(); + } +} diff --git a/src/app-server/structured-ephemeral-turn.ts b/src/app-server/structured-ephemeral-turn.ts index 13bfec75..9ff9365f 100644 --- a/src/app-server/structured-ephemeral-turn.ts +++ b/src/app-server/structured-ephemeral-turn.ts @@ -9,7 +9,6 @@ import type { RequestId } from "../generated/app-server/RequestId"; import type { ReasoningEffort } from "../generated/app-server/ReasoningEffort"; import type { ServerNotification } from "../generated/app-server/ServerNotification"; import type { JsonValue } from "../generated/app-server/serde_json/JsonValue"; -import type { ModelListResponse } from "../generated/app-server/v2/ModelListResponse"; import type { ThreadItem } from "../generated/app-server/v2/ThreadItem"; import type { ThreadStartResponse } from "../generated/app-server/v2/ThreadStartResponse"; import type { Turn } from "../generated/app-server/v2/Turn"; @@ -22,12 +21,11 @@ interface StructuredTurnRuntimeOverride { effort?: ReasoningEffort; } -type StructuredEphemeralTurnEvent = { type: "agent-message-delta"; delta: string } | { type: "reasoning-activity" }; +type StructuredTurnProgressEvent = { type: "agent-message-delta"; delta: string } | { type: "reasoning-activity" }; export interface StructuredEphemeralTurnClient { connect(): Promise; disconnect(): void; - listModels(includeHidden?: boolean): Promise; rejectServerRequest(requestId: RequestId, code: number, message: string): void; startEphemeralThread(cwd: string, serviceName: string, developerInstructions: string): Promise; startStructuredTurn( @@ -58,10 +56,9 @@ export interface RunStructuredEphemeralTurnOptions { exitedMessage: string; timedOutMessage: string; abortMessage?: string; - runtime?: StructuredTurnRuntimeOverride; + runtime?: StructuredTurnRuntimeOverride | undefined; signal?: AbortSignal | undefined; - onEvent?: (event: StructuredEphemeralTurnEvent) => void; - prepare?: ((client: StructuredEphemeralTurnClient) => Promise) | undefined; + onProgress?: (event: StructuredTurnProgressEvent) => void; clientFactory?: StructuredEphemeralTurnClientFactory | undefined; } @@ -83,7 +80,7 @@ export async function runStructuredEphemeralTurn(options: RunStructuredEphemeral handleNotification = (notification): void => { const result = transitionStructuredEphemeralTurnNotification(state, notification); state = result.state; - if (result.event) options.onEvent?.(result.event); + if (result.progress) options.onProgress?.(result.progress); if (result.completedTurn) resolve(result.completedTurn); }; }); @@ -107,9 +104,7 @@ export async function runStructuredEphemeralTurn(options: RunStructuredEphemeral try { await abortable(client.connect(), options.signal, options.abortMessage); - const runtime = options.prepare - ? await abortable(options.prepare(client), options.signal, options.abortMessage) - : (options.runtime ?? {}); + const runtime = options.runtime ?? {}; const threadResponse = await abortable( client.startEphemeralThread(options.cwd, options.serviceName, options.developerInstructions), options.signal, @@ -156,7 +151,7 @@ interface StructuredEphemeralTurnState { interface StructuredEphemeralTurnNotificationResult { state: StructuredEphemeralTurnState; - event?: StructuredEphemeralTurnEvent; + progress?: StructuredTurnProgressEvent; completedTurn?: Turn; } @@ -174,7 +169,7 @@ function transitionStructuredEphemeralTurnNotification( if (state.lifecycle.kind === "completed") return { state }; if (notification.method === "item/agentMessage/delta") { if (!structuredTurnRunMatches(state.lifecycle, notification.params.threadId, notification.params.turnId)) return { state }; - return { state, event: { type: "agent-message-delta", delta: notification.params.delta } }; + return { state, progress: { type: "agent-message-delta", delta: notification.params.delta } }; } if ( notification.method === "item/reasoning/summaryTextDelta" || @@ -182,7 +177,7 @@ function transitionStructuredEphemeralTurnNotification( notification.method === "item/reasoning/summaryPartAdded" ) { if (!structuredTurnRunMatches(state.lifecycle, notification.params.threadId, notification.params.turnId)) return { state }; - return { state, event: { type: "reasoning-activity" } }; + return { state, progress: { type: "reasoning-activity" } }; } if (notification.method === "item/completed") { if (!structuredTurnRunMatches(state.lifecycle, notification.params.threadId, notification.params.turnId)) return { state }; @@ -218,17 +213,12 @@ function abortable(promise: Promise, signal: AbortSignal | undefined, mess if (!signal) return promise; throwIfAborted(signal, message); return new Promise((resolve, reject) => { - const previousAbortHandler = signal.onabort; - const rejectAbort = (): void => { + const onAbort = (): void => { reject(structuredEphemeralTurnAbortError(message)); }; - const abortHandler = (event: Event): void => { - if (typeof previousAbortHandler === "function") previousAbortHandler.call(signal, event); - rejectAbort(); - }; - signal.onabort = abortHandler; + signal.addEventListener("abort", onAbort, { once: true }); promise.then(resolve, reject).finally(() => { - if (signal.onabort === abortHandler) signal.onabort = previousAbortHandler; + signal.removeEventListener("abort", onAbort); }); }); } diff --git a/src/features/selection-rewrite/runner.ts b/src/features/selection-rewrite/runner.ts index 1a01ee6e..09c3dcdb 100644 --- a/src/features/selection-rewrite/runner.ts +++ b/src/features/selection-rewrite/runner.ts @@ -5,7 +5,7 @@ import { type StructuredTurnOutputSchema, } from "../../app-server/structured-ephemeral-turn"; import type { ReasoningEffort } from "../../domain/catalog/metadata"; -import { panelModelOptionsFromAppServerModels } from "../../app-server/catalog-model"; +import { loadPanelModelOptions } from "../../app-server/model-options"; import type { PanelModelOption } from "../../domain/catalog/metadata"; import { runtimeOverride, validatedRuntimeOverrideForModelOptions } from "../../domain/catalog/runtime-overrides"; import type { SelectionRewriteRuntimeSettings } from "./model"; @@ -44,6 +44,9 @@ export type SelectionRewriteClientFactory = StructuredEphemeralTurnClientFactory export async function runSelectionRewrite(options: RunSelectionRewriteOptions): Promise { let preview = ""; const runtimeSettings = options.runtimeSettings; + const runtime = runtimeSettings + ? await selectionRewriteRuntimeOverrideForCodex(options.codexPath, options.cwd, runtimeSettings) + : undefined; const turn = await runStructuredEphemeralTurn({ codexPath: options.codexPath, cwd: options.cwd, @@ -57,9 +60,9 @@ export async function runSelectionRewrite(options: RunSelectionRewriteOptions): timedOutMessage: "Timed out while rewriting the selection.", abortMessage: "Selection rewrite cancelled.", signal: options.signal, - prepare: runtimeSettings ? (client) => selectionRewriteRuntimeOverrideForClient(client, runtimeSettings) : undefined, + runtime, clientFactory: options.clientFactory, - onEvent: (event) => { + onProgress: (event) => { if (event.type === "reasoning-activity") { options.onActivity?.("reasoning"); return; @@ -93,14 +96,15 @@ export function validatedSelectionRewriteRuntimeOverride( ); } -async function selectionRewriteRuntimeOverrideForClient( - client: SelectionRewriteClient, +async function selectionRewriteRuntimeOverrideForCodex( + codexPath: string, + cwd: string, settings: SelectionRewriteRuntimeSettings, ): Promise { const runtime = selectionRewriteRuntimeOverride(settings); if (!runtime.model || !runtime.effort) return runtime; try { - return validatedSelectionRewriteRuntimeOverride(settings, panelModelOptionsFromAppServerModels((await client.listModels(false)).data)); + return validatedSelectionRewriteRuntimeOverride(settings, await loadPanelModelOptions(codexPath, cwd, false)); } catch { return runtime; } diff --git a/src/workspace/short-lived-app-server-client.ts b/src/workspace/short-lived-app-server-client.ts index 9cd48d29..8998e13a 100644 --- a/src/workspace/short-lived-app-server-client.ts +++ b/src/workspace/short-lived-app-server-client.ts @@ -4,7 +4,6 @@ import { withShortLivedAppServerClient } from "../app-server/short-lived-client" export interface ShortLivedFallbackAppServerClientOptions { codexPath: string; cwd: string; - currentClient?: () => AppServerClient | null; unhandledServerRequestMessage: string; } @@ -12,9 +11,6 @@ export async function withShortLivedFallbackAppServerClient( options: ShortLivedFallbackAppServerClientOptions, operation: (client: AppServerClient) => Promise, ): Promise { - const currentClient = options.currentClient?.(); - if (currentClient?.isConnected()) return operation(currentClient); - return withShortLivedAppServerClient(options.codexPath, options.cwd, operation, { unhandledServerRequestMessage: options.unhandledServerRequestMessage, }); diff --git a/src/workspace/thread-title-generator.ts b/src/workspace/thread-title-generator.ts index 62530e3d..d54935db 100644 --- a/src/workspace/thread-title-generator.ts +++ b/src/workspace/thread-title-generator.ts @@ -1,10 +1,10 @@ -import { panelModelOptionsFromAppServerModels } from "../app-server/catalog-model"; import { runStructuredEphemeralTurn, type StructuredEphemeralTurnClient, type StructuredEphemeralTurnClientFactory, type StructuredTurnOutputSchema, } from "../app-server/structured-ephemeral-turn"; +import { loadPanelModelOptions } from "../app-server/model-options"; import type { PanelModelOption, ReasoningEffort } from "../domain/catalog/metadata"; import { runtimeOverride, validatedRuntimeOverrideForModelOptions } from "../domain/catalog/runtime-overrides"; import { namingPrompt, titleFromNamingTurn, type ThreadNamingContext } from "../domain/threads/naming"; @@ -48,6 +48,7 @@ export async function generateThreadTitleWithCodex( runtimeSettings: ThreadNamingRuntimeSettings, clientFactory?: ThreadNamingClientFactory, ): Promise { + const runtime = await threadNamingRuntimeOverrideForCodex(codexPath, cwd, runtimeSettings); const turn = await runStructuredEphemeralTurn({ codexPath, cwd, @@ -59,7 +60,7 @@ export async function generateThreadTitleWithCodex( unhandledServerRequestMessage: "Thread title generation does not handle server requests.", exitedMessage: "Codex title generation app-server exited.", timedOutMessage: "Timed out while generating a Codex thread title.", - prepare: (client) => threadNamingRuntimeOverrideForClient(client, runtimeSettings), + runtime, clientFactory, }); return titleFromNamingTurn(turn); @@ -81,15 +82,15 @@ export function validatedThreadNamingRuntimeOverride( return validatedRuntimeOverrideForModelOptions({ model: settings.threadNamingModel, effort: settings.threadNamingEffort }, models); } -async function threadNamingRuntimeOverrideForClient( - client: ThreadNamingClient, +async function threadNamingRuntimeOverrideForCodex( + codexPath: string, + cwd: string, settings: ThreadNamingRuntimeSettings, ): Promise { const runtime = threadNamingRuntimeOverride(settings); if (!runtime.model || !runtime.effort) return runtime; try { - const response = await client.listModels(false); - return validatedThreadNamingRuntimeOverride(settings, panelModelOptionsFromAppServerModels(response.data)); + return validatedThreadNamingRuntimeOverride(settings, await loadPanelModelOptions(codexPath, cwd, false)); } catch { return runtime; } diff --git a/tests/app-server/structured-ephemeral-turn.test.ts b/tests/app-server/structured-ephemeral-turn.test.ts new file mode 100644 index 00000000..d9d8bd37 --- /dev/null +++ b/tests/app-server/structured-ephemeral-turn.test.ts @@ -0,0 +1,273 @@ +// @vitest-environment jsdom + +import { describe, expect, it, vi } from "vitest"; + +import { + runStructuredEphemeralTurn, + type StructuredEphemeralTurnClient, + type StructuredEphemeralTurnClientFactory, +} from "../../src/app-server/structured-ephemeral-turn"; +import type { AppServerClientHandlers } from "../../src/app-server/client"; +import type { InitializeResponse } from "../../src/generated/app-server/InitializeResponse"; +import type { JsonValue } from "../../src/generated/app-server/serde_json/JsonValue"; +import type { RequestId } from "../../src/generated/app-server/RequestId"; +import type { ServerNotification } from "../../src/generated/app-server/ServerNotification"; +import type { ServerRequest } from "../../src/generated/app-server/ServerRequest"; +import type { ReasoningEffort } from "../../src/generated/app-server/ReasoningEffort"; +import type { Thread } from "../../src/generated/app-server/v2/Thread"; +import type { ThreadItem } from "../../src/generated/app-server/v2/ThreadItem"; +import type { ThreadStartResponse } from "../../src/generated/app-server/v2/ThreadStartResponse"; +import type { Turn } from "../../src/generated/app-server/v2/Turn"; +import type { TurnStartResponse } from "../../src/generated/app-server/v2/TurnStartResponse"; + +describe("runStructuredEphemeralTurn", () => { + it("fills completed turn items from item completion notifications", async () => { + const { clientFactory } = fakeStructuredTurnClientFactory((fake) => { + fake.startStructuredTurnImpl = async () => { + fake.emit(completedItemNotification("thread", "turn", agentMessage("answer", '{"ok":true}'))); + fake.emit(turnCompletedNotification("thread", turn([], { id: "turn", status: "completed" }))); + return { turn: turn([], { id: "turn", status: "inProgress" }) }; + }; + }); + + const result = await runStructuredEphemeralTurn(runOptions(clientFactory)); + + expect(result.items).toEqual([agentMessage("answer", '{"ok":true}')]); + expect(result.itemsView).toBe("full"); + }); + + it("reports matched structured turn progress without exposing raw notifications", async () => { + const progress: unknown[] = []; + const { clientFactory, client } = fakeStructuredTurnClientFactory(); + const running = runStructuredEphemeralTurn({ + ...runOptions(clientFactory), + onProgress: (event) => { + progress.push(event); + }, + }); + + await expectPresent(client.current).structuredTurnStarted; + const fake = expectPresent(client.current); + fake.emit(agentDeltaNotification("other-thread", "turn", "ignored")); + fake.emit(reasoningNotification("thread", "turn")); + fake.emit(agentDeltaNotification("thread", "turn", "draft")); + fake.emit(completedItemNotification("thread", "turn", agentMessage("answer", '{"ok":true}'))); + fake.emit(turnCompletedNotification("thread", turn([], { id: "turn", status: "completed" }))); + + await running; + expect(progress).toEqual([{ type: "reasoning-activity" }, { type: "agent-message-delta", delta: "draft" }]); + }); + + it("cleans up abort listeners after an operation settles", async () => { + const controller = new AbortController(); + const add = vi.spyOn(controller.signal, "addEventListener"); + const remove = vi.spyOn(controller.signal, "removeEventListener"); + const { clientFactory } = fakeStructuredTurnClientFactory((fake) => { + fake.startStructuredTurnImpl = async () => ({ turn: turn([agentMessage("answer", '{"ok":true}')]) }); + }); + + await runStructuredEphemeralTurn({ + ...runOptions(clientFactory), + signal: controller.signal, + }); + + expect(add).toHaveBeenCalledWith("abort", expect.any(Function), { once: true }); + expect(remove).toHaveBeenCalledTimes(add.mock.calls.length); + }); + + it("rejects server requests with the configured message", async () => { + const { clientFactory, client } = fakeStructuredTurnClientFactory((fake) => { + fake.connectImpl = async () => { + fake.request(serverRequest(123)); + return { codexHome: "/tmp/codex" } as InitializeResponse; + }; + fake.startStructuredTurnImpl = async () => ({ turn: turn([agentMessage("answer", '{"ok":true}')]) }); + }); + + await runStructuredEphemeralTurn(runOptions(clientFactory)); + + expect(expectPresent(client.current).rejectServerRequest).toHaveBeenCalledWith( + 123, + -32601, + "Structured test does not handle server requests.", + ); + }); +}); + +function runOptions(clientFactory: StructuredEphemeralTurnClientFactory): Parameters[0] { + return { + codexPath: "/bin/codex", + cwd: "/vault", + serviceName: "structured-test", + developerInstructions: "Return JSON.", + prompt: "Run.", + outputSchema: { type: "object" }, + timeoutMs: 10_000, + unhandledServerRequestMessage: "Structured test does not handle server requests.", + exitedMessage: "Structured test app-server exited.", + timedOutMessage: "Structured test timed out.", + clientFactory, + }; +} + +function fakeStructuredTurnClientFactory(configure?: (client: FakeStructuredTurnClient) => void): { + clientFactory: StructuredEphemeralTurnClientFactory; + client: { current: FakeStructuredTurnClient | null }; +} { + const client: { current: FakeStructuredTurnClient | null } = { current: null }; + return { + client, + clientFactory: (_codexPath, _cwd, handlers) => { + client.current = new FakeStructuredTurnClient(handlers); + configure?.(client.current); + return client.current; + }, + }; +} + +class FakeStructuredTurnClient implements StructuredEphemeralTurnClient { + connectImpl: (() => Promise) | null = null; + startStructuredTurnImpl: (() => Promise) | null = null; + readonly rejectServerRequest = vi.fn(); + readonly structuredTurnStarted: Promise; + private resolveStructuredTurnStarted!: () => void; + + constructor(private readonly handlers: AppServerClientHandlers) { + this.structuredTurnStarted = new Promise((resolve) => { + this.resolveStructuredTurnStarted = resolve; + }); + } + + async connect(): Promise { + return this.connectImpl ? this.connectImpl() : ({ codexHome: "/tmp/codex" } as InitializeResponse); + } + + disconnect(): void { + return undefined; + } + + async startEphemeralThread(): Promise { + return threadStartResponse("thread"); + } + + async startStructuredTurn( + _threadId: string, + _cwd: string, + _text: string, + _outputSchema: JsonValue, + _model?: string, + _effort?: ReasoningEffort, + ): Promise { + this.resolveStructuredTurnStarted(); + return this.startStructuredTurnImpl ? this.startStructuredTurnImpl() : { turn: turn([], { id: "turn", status: "inProgress" }) }; + } + + emit(notification: ServerNotification): void { + this.handlers.onNotification(notification); + } + + request(request: ServerRequest): void { + this.handlers.onServerRequest(request); + } +} + +function threadStartResponse(threadId: string): ThreadStartResponse { + return { + thread: thread(threadId), + model: "gpt-5.1", + modelProvider: "openai", + serviceTier: null, + cwd: "/vault", + runtimeWorkspaceRoots: [], + instructionSources: [], + approvalPolicy: "never", + approvalsReviewer: "auto_review", + sandbox: { type: "readOnly", networkAccess: false }, + activePermissionProfile: null, + reasoningEffort: null, + }; +} + +function thread(id: string): Thread { + return { + id, + sessionId: "session", + forkedFromId: null, + parentThreadId: null, + preview: "", + ephemeral: true, + modelProvider: "openai", + createdAt: 1, + updatedAt: 1, + status: { type: "idle" }, + path: null, + cwd: "/vault", + cliVersion: "0.0.0", + source: "appServer", + threadSource: null, + agentNickname: null, + agentRole: null, + gitInfo: null, + name: null, + turns: [], + }; +} + +function turn(items: Turn["items"], overrides: Partial = {}): Turn { + return { + id: "turn", + items, + itemsView: "full", + status: "completed", + error: null, + startedAt: null, + completedAt: null, + durationMs: null, + ...overrides, + }; +} + +function agentMessage(id: string, text: string): ThreadItem { + return { type: "agentMessage", id, text, phase: "final_answer", memoryCitation: null }; +} + +function completedItemNotification(threadId: string, turnId: string, item: ThreadItem): ServerNotification { + return { + method: "item/completed", + params: { threadId, turnId, item, completedAtMs: 1 }, + }; +} + +function turnCompletedNotification(threadId: string, completedTurn: Turn): ServerNotification { + return { + method: "turn/completed", + params: { threadId, turn: completedTurn }, + }; +} + +function agentDeltaNotification(threadId: string, turnId: string, delta: string): ServerNotification { + return { + method: "item/agentMessage/delta", + params: { threadId, turnId, itemId: "agent", delta }, + }; +} + +function reasoningNotification(threadId: string, turnId: string): ServerNotification { + return { + method: "item/reasoning/textDelta", + params: { threadId, turnId, itemId: "reasoning", contentIndex: 0, delta: "thinking" }, + }; +} + +function serverRequest(id: RequestId): ServerRequest { + return { + id, + method: "item/tool/requestUserInput", + params: {}, + } as ServerRequest; +} + +function expectPresent(value: T | null | undefined): T { + if (value == null) throw new Error("Expected value to be present."); + return value; +} diff --git a/tests/features/chat/threads/thread-naming.test.ts b/tests/features/chat/threads/thread-naming.test.ts index bb8e55b7..737d374f 100644 --- a/tests/features/chat/threads/thread-naming.test.ts +++ b/tests/features/chat/threads/thread-naming.test.ts @@ -23,7 +23,6 @@ import type { JsonValue } from "../../../../src/generated/app-server/serde_json/ import type { RequestId } from "../../../../src/generated/app-server/RequestId"; import type { ReasoningEffort } from "../../../../src/generated/app-server/ReasoningEffort"; import type { Model } from "../../../../src/generated/app-server/v2/Model"; -import type { ModelListResponse } from "../../../../src/generated/app-server/v2/ModelListResponse"; import type { ServerNotification } from "../../../../src/generated/app-server/ServerNotification"; import type { Thread } from "../../../../src/generated/app-server/v2/Thread"; import type { ThreadItem } from "../../../../src/generated/app-server/v2/ThreadItem"; @@ -330,10 +329,6 @@ class FakeThreadNamingClient implements ThreadNamingClient { return undefined; } - async listModels(): Promise { - return { data: [], nextCursor: null }; - } - rejectServerRequest(_requestId: RequestId, _code: number, _message: string): void { return undefined; } diff --git a/tests/features/selection-rewrite/selection-rewrite.test.ts b/tests/features/selection-rewrite/selection-rewrite.test.ts index 5919b725..65d2a1d7 100644 --- a/tests/features/selection-rewrite/selection-rewrite.test.ts +++ b/tests/features/selection-rewrite/selection-rewrite.test.ts @@ -28,7 +28,6 @@ import { import type { AppServerClientHandlers } from "../../../src/app-server/client"; import type { PanelModelOption } from "../../../src/domain/catalog/metadata"; import type { InitializeResponse } from "../../../src/generated/app-server/InitializeResponse"; -import type { ModelListResponse } from "../../../src/generated/app-server/v2/ModelListResponse"; import type { ServerNotification } from "../../../src/generated/app-server/ServerNotification"; import type { JsonValue } from "../../../src/generated/app-server/serde_json/JsonValue"; import type { RequestId } from "../../../src/generated/app-server/RequestId"; @@ -665,10 +664,6 @@ class FakeSelectionRewriteClient implements SelectionRewriteClient { this.disconnected = true; } - async listModels(): Promise { - return { data: [], nextCursor: null }; - } - rejectServerRequest(_requestId: RequestId, _code: number, _message: string): void { return undefined; }