fix(sessions): infer adapter from model id on local task start#2937
fix(sessions): infer adapter from model id on local task start#2937ashish921998 wants to merge 4 commits into
Conversation
When starting a local task with a GPT/Codex model selected, the local agent silently defaulted to Claude Opus if the adapter was missing. This adds inferAdapterFromModelId as a safety net in createNewLocalSession, reconnectToLocalSession, and clearSessionError. Cloud tasks were unaffected because the server validates the adapter independently. Co-authored-by: factory-droid[bot] <138933559+factory-droid[bot]@users.noreply.github.com>
|
Reviews (1): Last reviewed commit: "Merge branch 'main' into fix/local-codex..." | Re-trigger Greptile |
| export type TaskModelAdapter = "claude" | "codex"; | ||
|
|
||
| const CODEX_MODEL_PATTERNS = [/^gpt-/i, /^o\d/i, /^codex-/i]; | ||
| const CLAUDE_MODEL_PATTERNS = [/^claude-/i]; | ||
|
|
||
| export function inferAdapterFromModelId( | ||
| modelId: string | null | undefined, | ||
| ): TaskModelAdapter | undefined { |
There was a problem hiding this comment.
TaskModelAdapter is an exact duplicate of Adapter from @posthog/shared ("claude" | "codex"). Defining it separately violates OnceAndOnlyOnce — a future third adapter value would need to be added in two places, and callers in sessionService.ts already use Adapter as the expected type. Since @posthog/core already imports from @posthog/shared, the return type can use the canonical Adapter directly.
| export type TaskModelAdapter = "claude" | "codex"; | |
| const CODEX_MODEL_PATTERNS = [/^gpt-/i, /^o\d/i, /^codex-/i]; | |
| const CLAUDE_MODEL_PATTERNS = [/^claude-/i]; | |
| export function inferAdapterFromModelId( | |
| modelId: string | null | undefined, | |
| ): TaskModelAdapter | undefined { | |
| 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 { |
Note: If this suggestion doesn't match your team's coding style, reply to this and let me know. I'll remember it for next time!
| await this.createNewLocalSession( | ||
| taskId, | ||
| taskTitle, | ||
| repoPath, | ||
| authStatus.auth, | ||
| initialPrompt, | ||
| recoveredMode as ExecutionMode | undefined, | ||
| recoveredAdapter, | ||
| recoveredModel, | ||
| recoveredReasoning, | ||
| ); |
There was a problem hiding this comment.
The
currentValue from getConfigOptionByCategory is typed as string, so casting it directly to ExecutionMode is unsafe — any persisted value that doesn't match the union (e.g. a stale or renamed mode) would pass the cast silently and reach agent.start.mutate as an invalid permissionMode. The same pattern used for reasoningLevel — accept it as string and let the downstream call validate or ignore it — is safer here too.
| await this.createNewLocalSession( | |
| taskId, | |
| taskTitle, | |
| repoPath, | |
| authStatus.auth, | |
| initialPrompt, | |
| recoveredMode as ExecutionMode | undefined, | |
| recoveredAdapter, | |
| recoveredModel, | |
| recoveredReasoning, | |
| ); | |
| await this.createNewLocalSession( | |
| taskId, | |
| taskTitle, | |
| repoPath, | |
| authStatus.auth, | |
| initialPrompt, | |
| (recoveredMode as ExecutionMode | undefined) ?? undefined, | |
| recoveredAdapter, | |
| recoveredModel, | |
| recoveredReasoning, | |
| ); |
…ode recovery Replace duplicate TaskModelAdapter with Adapter from @posthog/shared, and use getCurrentModeFromConfigOptions instead of an unsafe ExecutionMode cast. Co-authored-by: factory-droid[bot] <138933559+factory-droid[bot]@users.noreply.github.com>
Problem
When starting a local task with a GPT/Codex model selected, the session always started with Claude Opus 4.8 instead of the selected model. Cloud tasks worked correctly.
Root Cause
The local agent process (
createAcpConnection) defaults to"claude"when no adapter is supplied:In
createNewLocalSession, theadapterandmodelwere forwarded independently. Whenadapterwasundefined(e.g. due to a race during adapter switching or a missing value), the local agent silently fell back to Claude regardless of the selected model. Themodelstring (e.g.gpt-5.5) was then passed to the Claude session, which couldn't resolve it and fell back toDEFAULT_GATEWAY_MODEL(claude-opus-4-8).Cloud tasks were unaffected because the PostHog server independently validates and enforces the adapter for the selected model.
Fix
Integrates
inferAdapterFromModelId(mapsgpt-*/o*/codex-*to"codex",claude-*to"claude") as a safety net in three places insessionService.ts:createNewLocalSession— infers the adapter from the model id when the caller did not supply one. Also avoids passing the Claude default model when the resolved adapter is Codex (Codex resolves its own default server-side viaAgent.run).reconnectToLocalSession— addsinferAdapterFromModelIdas a third-tier fallback (after the explicit adapter and the stored adapter) using the persisted model from config options.clearSessionError— now accepts optionaloverridesfor adapter/model/executionMode/reasoningLevel and recovers these from the errored session'sadapter+configOptionswhen no overrides are given. Also movesteardownSessionto aftercreateNewLocalSessionsucceeds, so a failed retry preserves the session placeholder.Testing
biome lint packages/corepasses with zeronoRestrictedImportsviolationstsc --noEmit --skipLibCheckpasses on all changed filesmodelAdapter.test.tsandclearSessionError.test.tsverified against implementation logic