diff --git a/packages/core/src/sessions/clearSessionError.test.ts b/packages/core/src/sessions/clearSessionError.test.ts new file mode 100644 index 000000000..dabc95aa3 --- /dev/null +++ b/packages/core/src/sessions/clearSessionError.test.ts @@ -0,0 +1,196 @@ +import type { AgentSession } from "@posthog/shared"; +import { describe, expect, it, vi } from "vitest"; +import { SessionService, type SessionServiceDeps } from "./sessionService"; + +const TASK_ID = "task-1"; +const RUN_ID = "run-1"; +const REPO = "/repo"; + +const noopLog = { + info: vi.fn(), + warn: vi.fn(), + error: vi.fn(), + debug: vi.fn(), +}; + +function makeSession(overrides: Partial = {}): AgentSession { + return { + taskRunId: RUN_ID, + taskId: TASK_ID, + taskTitle: "Task", + channel: "agent-event:run-1", + events: [], + startedAt: Date.now(), + status: "error", + isPromptPending: false, + isCompacting: false, + pendingPermissions: new Map(), + promptStartedAt: null, + pausedDurationMs: 0, + messageQueue: [], + optimisticItems: [], + initialPrompt: [{ type: "text", text: "do the thing" }], + ...overrides, + }; +} + +function createService(session?: AgentSession) { + const sessions: Record = {}; + if (session) sessions[session.taskRunId] = session; + + const store = { + getSessions: () => sessions, + getSessionByTaskId: (taskId: string) => + Object.values(sessions).find((s) => s.taskId === taskId), + setSession: (s: AgentSession) => { + sessions[s.taskRunId] = s; + }, + updateSession: (taskRunId: string, updates: Partial) => { + const s = sessions[taskRunId]; + if (s) Object.assign(s, updates); + }, + clearTailOptimisticItems: vi.fn(), + appendOptimisticItem: vi.fn(), + replaceOptimisticWithEvent: vi.fn(), + clearMessageQueue: vi.fn(), + }; + + const deps = { + store, + log: noopLog, + notifyPromptComplete: vi.fn(), + notifyPermissionRequest: vi.fn(), + getPersistedConfigOptions: () => undefined, + setPersistedConfigOptions: vi.fn(), + trpc: { + agent: { + onSessionIdleKilled: { + subscribe: () => ({ unsubscribe: vi.fn() }), + }, + cancel: { + mutate: vi.fn().mockResolvedValue(undefined), + }, + getPreviewConfigOptions: { + query: vi.fn().mockResolvedValue([]), + }, + }, + }, + } as unknown as SessionServiceDeps; + + const service = new SessionService(deps); + + // Replace the private helpers clearSessionError delegates to so the test can + // assert the session-intent args in isolation, without standing up the full + // agent.start/createTaskRun/subscribe chain inside createNewLocalSession. + const createSpy = vi.fn().mockResolvedValue(undefined); + const teardownSpy = vi.fn().mockResolvedValue(undefined); + const authSpy = vi.fn().mockResolvedValue({ + apiHost: "https://us.posthog.com", + projectId: 1, + client: {}, + }); + // biome-ignore lint/suspicious/noExplicitAny: spy on private methods + const anyService = service as any; + anyService.createNewLocalSession = createSpy; + anyService.teardownSession = teardownSpy; + anyService.getAuthCredentials = authSpy; + + return { service, createSpy, teardownSpy, sessions }; +} + +describe("clearSessionError preserves session intent on retry", () => { + it("forwards the caller overrides (adapter/model/executionMode/reasoningLevel)", async () => { + const { service, createSpy } = createService(makeSession()); + + await service.clearSessionError(TASK_ID, REPO, { + adapter: "codex", + model: "gpt-5.5", + executionMode: "auto", + reasoningLevel: "high", + }); + + expect(createSpy).toHaveBeenCalledTimes(1); + // createNewLocalSession(taskId, taskTitle, repoPath, auth, initialPrompt, + // executionMode, adapter, model, reasoningLevel) + const [, , , , , executionMode, adapter, model, reasoningLevel] = + createSpy.mock.calls[0]; + expect(adapter).toBe("codex"); + expect(model).toBe("gpt-5.5"); + expect(executionMode).toBe("auto"); + expect(reasoningLevel).toBe("high"); + }); + + it("recovers adapter/model/executionMode from a previously-live session when no overrides are given", async () => { + const session = makeSession({ + status: "error", + idleKilled: true, + adapter: "codex", + configOptions: [ + { + id: "model", + name: "Model", + type: "select", + currentValue: "gpt-5.5", + options: [], + category: "model", + }, + { + id: "mode", + name: "Approval Preset", + type: "select", + currentValue: "auto", + options: [], + category: "mode", + }, + { + id: "reasoning_effort", + name: "Reasoning Level", + type: "select", + currentValue: "high", + options: [], + category: "thought_level", + }, + ], + }); + const { service, createSpy } = createService(session); + + await service.clearSessionError(TASK_ID, REPO); + + expect(createSpy).toHaveBeenCalledTimes(1); + const [, , , , , executionMode, adapter, model, reasoningLevel] = + createSpy.mock.calls[0]; + expect(adapter).toBe("codex"); + expect(model).toBe("gpt-5.5"); + expect(executionMode).toBe("auto"); + expect(reasoningLevel).toBe("high"); + }); + + it("keeps the retry placeholder when fresh session creation fails", async () => { + const session = makeSession(); + const { service, createSpy, teardownSpy, sessions } = + createService(session); + createSpy.mockRejectedValueOnce(new Error("still broken")); + + await expect(service.clearSessionError(TASK_ID, REPO)).rejects.toThrow( + "still broken", + ); + + expect(teardownSpy).not.toHaveBeenCalled(); + expect(sessions[RUN_ID]).toBe(session); + }); + + it("recovers a Codex placeholder with no explicit model as Codex default", async () => { + const session = makeSession({ + adapter: "codex", + configOptions: [], + }); + const { service, createSpy } = createService(session); + + await service.clearSessionError(TASK_ID, REPO); + + expect(createSpy).toHaveBeenCalledTimes(1); + const [, , , , , , adapter, model] = createSpy.mock.calls[0]; + expect(adapter).toBe("codex"); + expect(model).toBeUndefined(); + }); +}); diff --git a/packages/core/src/sessions/sessionService.ts b/packages/core/src/sessions/sessionService.ts index 28993a584..523445f39 100644 --- a/packages/core/src/sessions/sessionService.ts +++ b/packages/core/src/sessions/sessionService.ts @@ -7,6 +7,7 @@ import type { SessionConfigOption, SessionUpdate, } from "@agentclientprotocol/sdk"; +import { inferAdapterFromModelId } from "@posthog/core/task-detail/modelAdapter"; import { type AcpMessage, type Adapter, @@ -17,6 +18,7 @@ import { getBackoffDelay, getCloudUrlFromRegion, getConfigOptionByCategory, + getCurrentModeFromConfigOptions, isFatalSessionError, isJsonRpcNotification, isJsonRpcRequest, @@ -831,9 +833,18 @@ export class SessionService { const events = convertStoredEntriesToEvents(rawEntries); const storedAdapter = this.d.adapterStore.getAdapter(taskRunId); - const resolvedAdapter = adapter ?? storedAdapter; const persistedConfigOptions = this.d.getPersistedConfigOptions(taskRunId); + // Infer the adapter from the persisted model as a safety net. A missing + // adapter on reconnect would cause the local agent to default to Claude + // even for Codex/OpenAI sessions. + const persistedModelForAdapter = + getConfigOptionByCategory(persistedConfigOptions, "model")?.currentValue; + const resolvedAdapter = + adapter ?? + storedAdapter ?? + inferAdapterFromModelId(persistedModelForAdapter); + const previous = this.d.store.getSessions()[taskRunId]; const session = createBaseSession(taskRunId, taskId, taskTitle); @@ -1207,7 +1218,20 @@ export class SessionService { } const { customInstructions: startCustomInstructions } = this.d.settings; - const preferredModel = model ?? this.d.DEFAULT_GATEWAY_MODEL; + + // Infer the adapter from the model id when the caller did not supply one. + // Without this, a GPT/Codex model paired with an undefined adapter causes + // the local agent process to default to Claude and silently ignore the + // selected model, falling back to the Claude default (Opus). + const resolvedAdapter = adapter ?? inferAdapterFromModelId(model); + + // Use the gateway default only when the caller did not pick a model AND the + // adapter is Claude (or still unknown). Codex resolves its own default + // server-side via Agent.run, so passing no model is correct there. + const preferredModel = + model ?? + (resolvedAdapter === "codex" ? undefined : this.d.DEFAULT_GATEWAY_MODEL); + const result = await this.d.trpc.agent.start.mutate({ taskId, taskRunId: taskRun.id, @@ -1215,7 +1239,7 @@ export class SessionService { apiHost: auth.apiHost, projectId: auth.projectId, permissionMode: executionMode, - adapter, + adapter: resolvedAdapter, customInstructions: startCustomInstructions || undefined, effort: effortLevelSchema.safeParse(reasoningLevel).success ? (reasoningLevel as EffortLevel) @@ -1226,7 +1250,7 @@ export class SessionService { const session = createBaseSession(taskRun.id, taskId, taskTitle); session.channel = result.channel; session.status = "connected"; - session.adapter = adapter; + session.adapter = resolvedAdapter; const configOptions = result.configOptions as | SessionConfigOption[] | undefined; @@ -1238,8 +1262,8 @@ export class SessionService { } // Persist the adapter - if (adapter) { - this.d.adapterStore.setAdapter(taskRun.id, adapter); + if (resolvedAdapter) { + this.d.adapterStore.setAdapter(taskRun.id, resolvedAdapter); } // Store the initial prompt on the session so retry/reset flows can @@ -1256,7 +1280,7 @@ export class SessionService { task_id: taskId, execution_type: "local", initial_mode: executionMode, - adapter, + adapter: resolvedAdapter, }); if (initialPrompt?.length) { @@ -3025,13 +3049,27 @@ export class SessionService { * initialPrompt saved from the original creation attempt), creates * a fresh session and re-sends the prompt instead of reconnecting * to an empty session. + * + * Optional overrides let callers (e.g. the "Retry" button) explicitly pin + * the adapter/model/mode for the fresh session. When no overrides are given + * the values are recovered from the errored session's adapter + + * configOptions so a retry does not silently revert to Claude/Opus. */ - async clearSessionError(taskId: string, repoPath: string): Promise { + async clearSessionError( + taskId: string, + repoPath: string, + overrides?: { + adapter?: Adapter; + model?: string; + executionMode?: ExecutionMode; + reasoningLevel?: string; + }, + ): Promise { this.localRepoPaths.set(taskId, repoPath); const session = this.d.store.getSessionByTaskId(taskId); if (session?.initialPrompt?.length) { const { taskTitle, initialPrompt } = session; - await this.teardownSession(session.taskRunId); + const oldTaskRunId = session.taskRunId; const authStatus = await this.getAuthCredentialsStatus(); if (authStatus.kind === "restoring") { throw new Error("Authentication is still restoring. Please wait."); @@ -3041,13 +3079,42 @@ export class SessionService { "Unable to reach server. Please check your connection.", ); } + + // Recover session intent from the errored session when the caller did + // not supply overrides, so retry preserves the user's original picks + // instead of resetting to defaults. + const recoveredModel = + overrides?.model ?? + getConfigOptionByCategory(session.configOptions, "model")?.currentValue; + const recoveredAdapter = + overrides?.adapter ?? + session.adapter ?? + inferAdapterFromModelId(recoveredModel); + const recoveredMode = + overrides?.executionMode ?? + getCurrentModeFromConfigOptions(session.configOptions); + const recoveredReasoning = + overrides?.reasoningLevel ?? + getConfigOptionByCategory( + session.configOptions, + "thought_level", + )?.currentValue; + + // Create the new session before tearing down the old one. If creation + // fails the caller can retry from the preserved placeholder instead of + // being left with no session at all. await this.createNewLocalSession( taskId, taskTitle, repoPath, authStatus.auth, initialPrompt, + recoveredMode, + recoveredAdapter, + recoveredModel, + recoveredReasoning, ); + await this.teardownSession(oldTaskRunId); return; } await this.reconnectInPlace(taskId, repoPath); diff --git a/packages/core/src/task-detail/modelAdapter.test.ts b/packages/core/src/task-detail/modelAdapter.test.ts new file mode 100644 index 000000000..c60062a46 --- /dev/null +++ b/packages/core/src/task-detail/modelAdapter.test.ts @@ -0,0 +1,25 @@ +import { describe, expect, it } from "vitest"; +import { inferAdapterFromModelId } from "./modelAdapter"; + +describe("inferAdapterFromModelId", () => { + it.each(["gpt-5.5", "gpt-5.4", "o3", "o4-mini", "codex-mini"])( + "maps OpenAI model %s to Codex", + (model) => { + expect(inferAdapterFromModelId(model)).toBe("codex"); + }, + ); + + it.each(["claude-opus-4-8", "claude-sonnet-4-5"])( + "maps Anthropic model %s to Claude", + (model) => { + expect(inferAdapterFromModelId(model)).toBe("claude"); + }, + ); + + it.each([undefined, null, "", "fable", "custom-model"])( + "leaves ambiguous model %s unchanged", + (model) => { + expect(inferAdapterFromModelId(model)).toBeUndefined(); + }, + ); +}); diff --git a/packages/core/src/task-detail/modelAdapter.ts b/packages/core/src/task-detail/modelAdapter.ts new file mode 100644 index 000000000..714d8599f --- /dev/null +++ b/packages/core/src/task-detail/modelAdapter.ts @@ -0,0 +1,20 @@ +import type { Adapter } from "@posthog/shared"; + +const CODEX_MODEL_PATTERNS = [/^gpt-/i, /^o\d/i, /^codex-/i]; +const CLAUDE_MODEL_PATTERNS = [/^claude-/i]; + +export function inferAdapterFromModelId( + modelId: string | null | undefined, +): Adapter | undefined { + if (!modelId) return undefined; + const normalized = modelId.trim(); + if (!normalized) return undefined; + + if (CODEX_MODEL_PATTERNS.some((pattern) => pattern.test(normalized))) { + return "codex"; + } + if (CLAUDE_MODEL_PATTERNS.some((pattern) => pattern.test(normalized))) { + return "claude"; + } + return undefined; +}