From f0a2e5417b759db93540ee2b96fb9c5707c9f417 Mon Sep 17 00:00:00 2001 From: Andrew Maguire Date: Wed, 17 Jun 2026 10:09:06 +0100 Subject: [PATCH] fix(inbox): validate persisted model before one-click cloud tasks MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit One-click cloud-task flows (scout chat, Responder setup, home quick actions, Discuss/Create-PR) used the persisted `lastUsedModel` directly without checking it was still offered by the gateway. A stale id — e.g. a model later de-listed for the org — would be sent verbatim and the run failed with a gateway 403. The TaskInput model picker already guards this and silently falls back to the server default, which is why the picker showed Opus while the scout still tried the de-listed model. Centralise the guard in the model resolver: the persisted/pinned model is now passed as a *preference* and only honoured when it appears in the gateway's current options, otherwise the server default is used. Generated-By: PostHog Code Task-Id: f29142f6-4183-48a6-9046-8cba7f8181de --- apps/code/src/renderer/desktop-services.ts | 3 +- packages/core/src/inbox/identifiers.ts | 6 ++ .../core/src/inbox/reportTaskCreation.test.ts | 56 +++++++++++++++++++ packages/core/src/inbox/reportTaskCreation.ts | 29 +++++++++- .../src/inbox/signalReportTaskService.test.ts | 15 +++++ .../core/src/inbox/signalReportTaskService.ts | 10 +++- .../home/hooks/useRunWorkstreamAction.ts | 10 +++- .../components/ConfigureAgentsSection.tsx | 15 +++-- .../inbox/hooks/resolveDefaultModel.ts | 22 +++++--- .../inbox/hooks/useInboxCloudTaskRunner.ts | 14 ++++- 10 files changed, 153 insertions(+), 27 deletions(-) create mode 100644 packages/core/src/inbox/reportTaskCreation.test.ts 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);