diff --git a/apps/code/src/renderer/desktop-services.ts b/apps/code/src/renderer/desktop-services.ts index a5dd914288..02fb9ddaa3 100644 --- a/apps/code/src/renderer/desktop-services.ts +++ b/apps/code/src/renderer/desktop-services.ts @@ -108,13 +108,14 @@ container.bind(REPORT_MODEL_RESOLVER).toConstantValue({ async resolveDefaultModel( apiHost: string, adapter: "claude" | "codex", + preferredModel?: string | null, ): Promise { try { const options = await hostTrpcClient.agent.getPreviewConfigOptions.query({ apiHost, adapter, }); - return selectModelFromOptions(options); + return selectModelFromOptions(options, preferredModel); } catch (error) { reportModelResolverLog.warn("Failed to resolve default model", { error, diff --git a/packages/core/src/inbox/identifiers.ts b/packages/core/src/inbox/identifiers.ts index 3d515f0be0..31aba20b2d 100644 --- a/packages/core/src/inbox/identifiers.ts +++ b/packages/core/src/inbox/identifiers.ts @@ -18,9 +18,15 @@ export const LINEAR_OAUTH_FLOW = Symbol.for( ); export interface ReportModelResolver { + /** + * Resolve the model id to use for a cloud task. `preferredModel` (e.g. the + * persisted last-used model) is honoured only if the gateway still offers it; + * otherwise the adapter's server default is returned. + */ resolveDefaultModel( apiHost: string, adapter: "claude" | "codex", + preferredModel?: string | null, ): Promise; } diff --git a/packages/core/src/inbox/reportTaskCreation.test.ts b/packages/core/src/inbox/reportTaskCreation.test.ts new file mode 100644 index 0000000000..bb65901149 --- /dev/null +++ b/packages/core/src/inbox/reportTaskCreation.test.ts @@ -0,0 +1,56 @@ +import { describe, expect, it } from "vitest"; +import { + type PreviewConfigOption, + selectModelFromOptions, +} from "./reportTaskCreation"; + +function modelOption( + currentValue: string, + available: string[], +): PreviewConfigOption { + return { + id: "model", + category: "model", + type: "select", + currentValue, + options: available.map((value) => ({ value })), + }; +} + +describe("selectModelFromOptions", () => { + it("returns the server default when no preferred model is given", () => { + const options = [modelOption("claude-opus-4-8", ["claude-opus-4-8"])]; + expect(selectModelFromOptions(options)).toBe("claude-opus-4-8"); + }); + + it("honours the preferred model when the gateway still offers it", () => { + const options = [ + modelOption("claude-opus-4-8", ["claude-opus-4-8", "claude-sonnet-4-6"]), + ]; + expect(selectModelFromOptions(options, "claude-sonnet-4-6")).toBe( + "claude-sonnet-4-6", + ); + }); + + it("falls back to the server default when the preferred model is no longer offered", () => { + // The persisted model (e.g. a de-listed fable) is not in the available + // options, so it must not be returned — otherwise the run 403s. + const options = [modelOption("claude-opus-4-8", ["claude-opus-4-8"])]; + expect(selectModelFromOptions(options, "claude-fable-5")).toBe( + "claude-opus-4-8", + ); + }); + + it("ignores an empty preferred model", () => { + const options = [modelOption("claude-opus-4-8", ["claude-opus-4-8"])]; + expect(selectModelFromOptions(options, "")).toBe("claude-opus-4-8"); + expect(selectModelFromOptions(options, null)).toBe("claude-opus-4-8"); + }); + + it("returns undefined when there is no model option", () => { + const options: PreviewConfigOption[] = [ + { id: "mode", category: "mode", type: "select", currentValue: "plan" }, + ]; + expect(selectModelFromOptions(options, "claude-opus-4-8")).toBeUndefined(); + }); +}); diff --git a/packages/core/src/inbox/reportTaskCreation.ts b/packages/core/src/inbox/reportTaskCreation.ts index cdce6d812c..84864ce407 100644 --- a/packages/core/src/inbox/reportTaskCreation.ts +++ b/packages/core/src/inbox/reportTaskCreation.ts @@ -1,22 +1,47 @@ import type { TaskCreationInput } from "@posthog/shared"; +/** A selectable choice, either flat or wrapped in a labelled group. */ +export interface PreviewConfigChoice { + value?: string; + options?: PreviewConfigChoice[]; +} + /** Minimal shape of a preview-config option we scan for the default model. */ export interface PreviewConfigOption { id?: string; category?: string; type?: string; currentValue?: string | boolean | null; + options?: PreviewConfigChoice[]; } -/** Pick the default model id out of the agent's preview-config options, if present. */ +/** + * Pick the model id out of the agent's preview-config options. + * + * When `preferredModel` is supplied (e.g. the user's persisted last-used model) + * it is honoured only if it is still one of the gateway's available models; + * otherwise we fall back to the server default (`currentValue`). One-click cloud + * flows pass their persisted model here so a stale id the gateway no longer + * offers can't slip through — without the check the run fails with a gateway 403 + * (e.g. a previously-selected model that was later de-listed for the org). + */ export function selectModelFromOptions( options: PreviewConfigOption[], + preferredModel?: string | null, ): string | undefined { const modelOption = options.find( (o) => o.id === "model" || o.category === "model", ); + if (modelOption?.type !== "select") { + return undefined; + } + if ( + preferredModel && + modelOption.options?.some((o) => o.value === preferredModel) + ) { + return preferredModel; + } if ( - modelOption?.type === "select" && typeof modelOption.currentValue === "string" && modelOption.currentValue ) { diff --git a/packages/core/src/inbox/signalReportTaskService.test.ts b/packages/core/src/inbox/signalReportTaskService.test.ts index 6e1cf43e6a..a3fb539817 100644 --- a/packages/core/src/inbox/signalReportTaskService.test.ts +++ b/packages/core/src/inbox/signalReportTaskService.test.ts @@ -76,6 +76,21 @@ describe("SignalReportTaskService", () => { expect(result.status).toBe("created"); }); + it("forwards the override to the resolver as a preference, not a hard selection", async () => { + // The resolver validates the override against the gateway's available + // models, so it must receive it rather than the service short-circuiting. + const { service, modelResolver } = makeService(); + await service.createSignalReportTask( + makeInput({ modelOverride: "claude-sonnet" }), + vi.fn(), + ); + expect(modelResolver.resolveDefaultModel).toHaveBeenCalledWith( + expect.any(String), + "claude", + "claude-sonnet", + ); + }); + it("aborts with missing-model when no model can be resolved", async () => { const { service, createTask } = makeService( {}, diff --git a/packages/core/src/inbox/signalReportTaskService.ts b/packages/core/src/inbox/signalReportTaskService.ts index 71fe958f57..edd6d22ace 100644 --- a/packages/core/src/inbox/signalReportTaskService.ts +++ b/packages/core/src/inbox/signalReportTaskService.ts @@ -66,9 +66,13 @@ export class SignalReportTaskService { } const apiHost = getCloudUrlFromRegion(input.cloudRegion); - const model = - input.modelOverride ?? - (await this.modelResolver.resolveDefaultModel(apiHost, input.adapter)); + // The override is a preference: the resolver keeps it only if the gateway + // still offers it, otherwise it falls back to the server default. + const model = await this.modelResolver.resolveDefaultModel( + apiHost, + input.adapter, + input.modelOverride, + ); if (!model) { return { status: "missing-model" }; } diff --git a/packages/ui/src/features/home/hooks/useRunWorkstreamAction.ts b/packages/ui/src/features/home/hooks/useRunWorkstreamAction.ts index 8eacc466fb..3c1f4df596 100644 --- a/packages/ui/src/features/home/hooks/useRunWorkstreamAction.ts +++ b/packages/ui/src/features/home/hooks/useRunWorkstreamAction.ts @@ -99,13 +99,17 @@ export function useRunWorkstreamAction() { void (async () => { try { // The cloud runtime requires a model: action-pinned, then last-used, - // then the adapter's server default. + // then the adapter's server default. The preferred candidate is only + // honoured if the gateway still offers it (the resolver validates it), + // so a stale persisted/pinned id can't reach the run and 403. const adapter = action.adapter ?? lastUsedAdapter; - let model = action.model ?? lastUsedModel ?? undefined; - if (!model && cloudRegion) { + const preferredModel = action.model ?? lastUsedModel ?? undefined; + let model = preferredModel; + if (cloudRegion) { model = await modelResolver.resolveDefaultModel( getCloudUrlFromRegion(cloudRegion), adapter, + preferredModel, ); } if (!model) { diff --git a/packages/ui/src/features/inbox/components/ConfigureAgentsSection.tsx b/packages/ui/src/features/inbox/components/ConfigureAgentsSection.tsx index 5456cf67a5..7d1904c390 100644 --- a/packages/ui/src/features/inbox/components/ConfigureAgentsSection.tsx +++ b/packages/ui/src/features/inbox/components/ConfigureAgentsSection.tsx @@ -301,14 +301,13 @@ function SetupTaskSection() { const settings = useSettingsStore.getState(); const adapter = settings.lastUsedAdapter ?? "claude"; const apiHost = getCloudUrlFromRegion(cloudRegion); - const model = - settings.lastUsedModel ?? - (await resolveDefaultModel( - queryClient, - apiHost, - adapter, - modelResolver, - )); + const model = await resolveDefaultModel( + queryClient, + apiHost, + adapter, + modelResolver, + settings.lastUsedModel, + ); if (!model) { sonnerToast.dismiss(toastId); diff --git a/packages/ui/src/features/inbox/hooks/resolveDefaultModel.ts b/packages/ui/src/features/inbox/hooks/resolveDefaultModel.ts index ae91a12d03..63d3993b6b 100644 --- a/packages/ui/src/features/inbox/hooks/resolveDefaultModel.ts +++ b/packages/ui/src/features/inbox/hooks/resolveDefaultModel.ts @@ -5,23 +5,31 @@ import type { QueryClient } from "@tanstack/react-query"; const log = logger.scope("resolve-default-model"); /** - * Resolve the default model for the given adapter via the preview-config - * tRPC query. Returns the server's `currentValue` for the `model` option, or - * undefined if the call fails or the option is missing. + * Resolve the model for the given adapter via the preview-config tRPC query. * - * Used by inbox flows that create cloud tasks directly (Discuss, Create PR) - * without going through TaskInput – they need a model to pass to the saga - * and the user hasn't necessarily picked one yet. + * `preferredModel` (e.g. the persisted last-used model) is honoured only if the + * gateway still offers it; otherwise the server default (`currentValue`) is + * returned. Returns undefined if the call fails or the option is missing. + * + * Used by one-click flows that create cloud tasks directly (Discuss, Create PR, + * scout chat) without going through TaskInput – they need a model to pass to the + * saga. Validating the preferred model here is what keeps a stale persisted id + * the gateway no longer offers from reaching the run and 403-ing. */ export async function resolveDefaultModel( queryClient: QueryClient, apiHost: string, adapter: "claude" | "codex", modelResolver: ReportModelResolver, + preferredModel?: string | null, ): Promise { void queryClient; try { - return await modelResolver.resolveDefaultModel(apiHost, adapter); + return await modelResolver.resolveDefaultModel( + apiHost, + adapter, + preferredModel, + ); } catch (error) { log.warn("Failed to resolve default model", { error, adapter }); } diff --git a/packages/ui/src/features/inbox/hooks/useInboxCloudTaskRunner.ts b/packages/ui/src/features/inbox/hooks/useInboxCloudTaskRunner.ts index eff1720415..f8d76e6940 100644 --- a/packages/ui/src/features/inbox/hooks/useInboxCloudTaskRunner.ts +++ b/packages/ui/src/features/inbox/hooks/useInboxCloudTaskRunner.ts @@ -135,9 +135,17 @@ export function useInboxCloudTaskRunner({ const adapter = settings.lastUsedAdapter ?? "claude"; const apiHost = getCloudUrlFromRegion(cloudRegion); - const model = - settings.lastUsedModel ?? - (await resolveDefaultModel(queryClient, apiHost, adapter, modelResolver)); + // Pass the persisted model as a *preference*, not a hard selection: the + // resolver keeps it only if the gateway still offers it, otherwise it falls + // back to the server default. A stale id (e.g. one later de-listed for the + // org) would otherwise be sent here and fail the run with a gateway 403. + const model = await resolveDefaultModel( + queryClient, + apiHost, + adapter, + modelResolver, + settings.lastUsedModel, + ); if (!model) { sonnerToast.dismiss(toastId);