fix(site): harden organization-scoped model loading (#28495)

Previously, the new agent form treated the dashboard organization as a
usable resource scope while permission filtering was still pending. This
could request a provisional empty model catalog, briefly show the
no-model setup notice, and lose the selected organization when the form
remounted. A malformed organization models response could also crash the
Models settings page.

Wait for authoritative permitted organizations before enabling
organization-scoped resources, persist valid organization selections
across remounts, and reject malformed model responses while retaining a
defensive rendering guard.

## UAT

An adversarial [Coder Agents UAT
run](https://dev.coder.com/agents/980a5a6d-e91b-47f0-902e-25d12ac46935)
validated this commit on a licensed local deployment with two
organizations, a real Anthropic model, and a non-admin user. Under
throttled and artificially delayed authorization, no organization-scoped
request fired before permissions settled and the false setup notice
never appeared, while an authorized zero-model organization still showed
the genuine notice. Persistence across reloads, tampered or stale
localStorage IDs, membership revocation, attachment-preserving
organization switches, single-organization deployments, and induced
malformed or failed model responses on the Models settings page all
behaved correctly. Verdict: PASS.

site/src/api/api.test.ts

@@ -451,6 +451,18 @@ describe("api.ts", () => {
 			expect(result).toStrictEqual(responseData);
 		});
 
+		it("rejects chat model responses without a models array", async () => {
+			vi.spyOn(axiosInstance, "get").mockResolvedValueOnce({
+				data: { providers: [], unsupported_providers: [] },
+			});
+
+			await expect(
+				API.experimental.getChatModels(organizationId),
+			).rejects.toThrow(
+				"Invalid chat models response: models must be an array.",
+			);
+		});
+
 		it.each<[string, () => Promise<unknown>]>([
 			[
 				"/api/experimental/organizations/organization%2Fid/chats/models",

site/src/api/api.ts

@@ -3903,6 +3903,9 @@ class ExperimentalApiMethods {
 			await this.axios.get<TypesGen.OrganizationChatModelsResponse>(
 				chatModelsPath(organizationId),
 			);
+		if (!Array.isArray(response.data?.models)) {
+			throw new Error("Invalid chat models response: models must be an array.");
+		}
 		return response.data;
 	};
 

site/src/pages/AISettingsPage/ModelsPage/OrganizationModelsLayout.stories.tsx

@@ -358,6 +358,24 @@ export const PermissionLoadErrorShowsAlert: Story = {
 	},
 };
 
+export const ModelLoadErrorShowsAlert: Story = {
+	beforeEach: () => {
+		spyOn(API.experimental, "getChatModels").mockRejectedValue(
+			new Error("Invalid chat models response: models must be an array."),
+		);
+		spyOn(API, "checkAuthorization").mockResolvedValue({});
+	},
+	parameters: { queries: [] },
+	play: async ({ canvasElement }) => {
+		const canvas = within(canvasElement);
+		expect(
+			await canvas.findByText(
+				"Invalid chat models response: models must be an array.",
+			),
+		).toBeVisible();
+	},
+};
+
 export const Loading: Story = {
 	beforeEach: () => {
 		spyOn(API.experimental, "getChatModels").mockImplementation(

site/src/pages/AISettingsPage/ModelsPage/organizationModels.tsx

@@ -29,7 +29,7 @@ export const useAccessibleModelOrganizations = (
 	);
 	const accessibleOrganizations = organizations.filter(
 		(organization, index) =>
-			(queries[index]?.data?.models.length ?? 0) > 0 ||
+			(queries[index]?.data?.models?.length ?? 0) > 0 ||
 			canAccessOrganizationChatModelConfig(
 				permissionsQuery.data?.[organization.id],
 			),

site/src/pages/AgentsPage/components/AgentCreateForm.stories.tsx

@@ -1,6 +1,6 @@
 import type { Decorator, Meta, StoryObj } from "@storybook/react-vite";
 import { delay } from "msw";
-import { useEffect, useState } from "react";
+import { type ComponentProps, useEffect, useState } from "react";
 import { QueryClient, QueryClientProvider, useQueryClient } from "react-query";
 import {
 	expect,
@@ -26,6 +26,7 @@ import {
 	MockChatModel,
 	MockChatModelProviderDescriptor,
 } from "#/testHelpers/chatModels";
+import { createDeferred, type Deferred } from "#/testHelpers/deferred";
 import {
 	MockDefaultOrganization,
 	MockOrganization2,
@@ -37,8 +38,15 @@ import {
 	getReasoningEffortForModel,
 	saveReasoningEffortForModel,
 } from "../utils/reasoningEffort";
-import { AgentCreateForm, emptyInputStorageKey } from "./AgentCreateForm";
+import {
+	AgentCreateForm,
+	emptyInputStorageKey,
+	selectedOrganizationIdStorageKey,
+} from "./AgentCreateForm";
 
+let pendingOrganizationAuthorization: Deferred<
+	Awaited<ReturnType<typeof API.checkAuthorization>>
+>;
 let capturedQueryClient: QueryClient | undefined;
 
 const permittedOrgsKey = permittedOrganizationsKey({
@@ -298,6 +306,20 @@ type Story = StoryObj<typeof AgentCreateForm>;
 
 const defaultArgs = meta.args;
 
+const RemountAgentCreateForm = (
+	props: ComponentProps<typeof AgentCreateForm>,
+) => {
+	const [key, setKey] = useState(0);
+	return (
+		<>
+			<button type="button" onClick={() => setKey((current) => current + 1)}>
+				Remount form
+			</button>
+			<AgentCreateForm key={key} {...props} />
+		</>
+	);
+};
+
 const mockPermittedOrganizations = (
 	permissions: Record<string, boolean>,
 	delayMs = 0,
@@ -1644,6 +1666,7 @@ export const OrganizationAuthorizationFailure: Story = {
 	parameters: {
 		showOrganizations: true,
 		organizations: [MockDefaultOrganization, MockOrganization2],
+		queries: [],
 	},
 	beforeEach: () => {
 		localStorage.clear();
@@ -1655,10 +1678,18 @@ export const OrganizationAuthorizationFailure: Story = {
 		spyOn(API, "checkAuthorization").mockRejectedValue(
 			new Error("authorization check failed"),
 		);
+		spyOn(API.experimental, "getChatModels").mockResolvedValue(
+			defaultModelCatalog,
+		);
 	},
 	play: async ({ canvasElement }) => {
 		const canvas = within(canvasElement);
 		await canvas.findAllByText(/authorization check failed/i);
+		expect(API.experimental.getChatModels).not.toHaveBeenCalled();
+		expect(API.experimental.getMCPServerConfigs).not.toHaveBeenCalled();
+		expect(
+			API.experimental.getUserChatPersonalModelOverrides,
+		).not.toHaveBeenCalled();
 		expect(canvas.getByRole("button", { name: "Send" })).toBeDisabled();
 	},
 };
@@ -1695,15 +1726,23 @@ export const DelayedOrganizationAuthorization: Story = {
 	parameters: {
 		showOrganizations: true,
 		organizations: [MockDefaultOrganization, MockOrganization2],
+		queries: [],
 	},
 	beforeEach: () => {
 		localStorage.setItem(emptyInputStorageKey, "draft message");
-		mockPermittedOrganizations(
-			{
-				[MockDefaultOrganization.id]: true,
-				[MockOrganization2.id]: true,
-			},
-			1_500,
+		pendingOrganizationAuthorization = createDeferred();
+		spyOn(API, "getOrganizations").mockResolvedValue([
+			MockDefaultOrganization,
+			MockOrganization2,
+		]);
+		spyOn(API, "checkAuthorization").mockImplementation(
+			() => pendingOrganizationAuthorization.promise,
+		);
+		spyOn(API.experimental, "getChatModels").mockImplementation(
+			async (organizationId) =>
+				organizationId === MockOrganization2.id
+					? organization2LocalCatalog
+					: { ...defaultModelCatalog, models: [] },
 		);
 	},
 	args: {
@@ -1724,6 +1763,15 @@ export const DelayedOrganizationAuthorization: Story = {
 		await expect(
 			canvas.getByRole("button", { name: "More options" }),
 		).toBeDisabled();
+		expect(API.experimental.getChatModels).not.toHaveBeenCalled();
+		expect(API.experimental.getMCPServerConfigs).not.toHaveBeenCalled();
+		expect(
+			API.experimental.getUserChatPersonalModelOverrides,
+		).not.toHaveBeenCalled();
+		expect(
+			canvas.queryByText(/AI models aren't available yet/i),
+		).not.toBeInTheDocument();
+		expect(canvas.queryByText("No model is available")).not.toBeInTheDocument();
 		// dispatchEvent returns false when a handler accepted the drop
 		// via preventDefault, giving a race-free accepted/ignored signal.
 		const dropFile = (name: string): boolean => {
@@ -1741,12 +1789,28 @@ export const DelayedOrganizationAuthorization: Story = {
 		// must be ignored.
 		expect(dropFile("drop.txt")).toBe(true);
 		expect(canvas.queryByLabelText("Remove drop.txt")).not.toBeInTheDocument();
-		// The pending option list is unfiltered, so the picker must stay hidden.
 		expect(
 			canvas.queryByRole("button", { name: /organization/i }),
 		).not.toBeInTheDocument();
-		await waitFor(() => expect(sendButton).toBeEnabled(), { timeout: 3_000 });
-		await canvas.findByRole("button", { name: /organization/i });
+
+		pendingOrganizationAuthorization.resolve({
+			[MockDefaultOrganization.id]: false,
+			[MockOrganization2.id]: true,
+		});
+
+		await waitFor(() => expect(sendButton).toBeEnabled());
+		expect(API.experimental.getChatModels).toHaveBeenCalledWith(
+			MockOrganization2.id,
+		);
+		expect(API.experimental.getChatModels).not.toHaveBeenCalledWith(
+			MockDefaultOrganization.id,
+		);
+		expect(API.experimental.getMCPServerConfigs).toHaveBeenCalledWith(
+			MockOrganization2.id,
+		);
+		expect(
+			API.experimental.getUserChatPersonalModelOverrides,
+		).toHaveBeenCalledWith(MockOrganization2.id, "me");
 		// Positive control: once settled the same drop is accepted and
 		// attaches, so the pending-state assertions exercised a real path.
 		expect(dropFile("after.txt")).toBe(false);
@@ -1756,6 +1820,53 @@ export const DelayedOrganizationAuthorization: Story = {
 	},
 };
 
+export const SelectedOrganizationSurvivesRemount: Story = {
+	parameters: {
+		showOrganizations: true,
+		organizations: [MockDefaultOrganization, MockOrganization2],
+		queries: [],
+	},
+	args: { ...defaultArgs },
+	render: (args) => <RemountAgentCreateForm {...args} />,
+	beforeEach: () => {
+		mockPermittedOrganizations({
+			[MockDefaultOrganization.id]: true,
+			[MockOrganization2.id]: true,
+		});
+		spyOn(API.experimental, "getChatModels").mockImplementation(
+			async (organizationId) =>
+				organizationId === MockOrganization2.id
+					? organization2LocalCatalog
+					: defaultModelCatalog,
+		);
+	},
+	play: async ({ canvasElement }) => {
+		const canvas = within(canvasElement);
+		await userEvent.click(
+			await canvas.findByRole("button", {
+				name: `Organization: ${MockDefaultOrganization.display_name}`,
+			}),
+		);
+		await userEvent.click(
+			await screen.findByRole("option", {
+				name: MockOrganization2.display_name,
+			}),
+		);
+		await waitFor(() => {
+			expect(localStorage.getItem(selectedOrganizationIdStorageKey)).toBe(
+				MockOrganization2.id,
+			);
+		});
+
+		await userEvent.click(canvas.getByRole("button", { name: "Remount form" }));
+		expect(
+			await canvas.findByRole("button", {
+				name: `Organization: ${MockOrganization2.display_name}`,
+			}),
+		).toBeVisible();
+	},
+};
+
 // Mutable permissions let play functions change authorization across refetches.
 // The story-local QueryClient exposes those refetches; the preview client's
 // instance is inaccessible and uses infinite stale time.

site/src/pages/AgentsPage/components/AgentCreateForm.tsx

@@ -48,6 +48,9 @@ import { getModelSelectorHelp } from "./ModelSelectorHelp";
 
 /** @internal Exported for testing. */
 export const emptyInputStorageKey = "agents.empty-input";
+/** @internal Exported for testing. */
+export const selectedOrganizationIdStorageKey =
+	"agents.selected-organization-id";
 const selectedWorkspaceIdStorageKey = "agents.selected-workspace-id";
 const lastModelConfigIDStorageKey = "agents.last-model-config-id";
 
@@ -165,8 +168,6 @@ export const AgentCreateForm: FC<AgentCreateFormProps> = ({
 	const [initialLastModelConfigID] = useState(() => {
 		return localStorage.getItem(lastModelConfigIDStorageKey) ?? "";
 	});
-	const initialOrg =
-		organizations.find((o) => o.is_default) ?? organizations[0];
 	// effectiveWorkspaceId nulls a stored selection outside the effective org's
 	// filtered workspace list without deleting it. Preserve the stored value
 	// because the permitted-organizations query may resolve after mount and
@@ -175,7 +176,16 @@ export const AgentCreateForm: FC<AgentCreateFormProps> = ({
 		() => localStorage.getItem(selectedWorkspaceIdStorageKey),
 	);
 	const [selectedOrg, setSelectedOrg] = useState<TypesGen.Organization | null>(
-		null,
+		() => {
+			const storedOrganizationId = localStorage.getItem(
+				selectedOrganizationIdStorageKey,
+			);
+			return (
+				organizations.find(
+					(organization) => organization.id === storedOrganizationId,
+				) ?? null
+			);
+		},
 	);
 	const [pendingOrgChange, setPendingOrgChange] =
 		useState<TypesGen.Organization | null>(null);
@@ -195,14 +205,13 @@ export const AgentCreateForm: FC<AgentCreateFormProps> = ({
 	// selection, its organization list is authoritative so a removed org cannot
 	// remain selected for submission.
 	const permittedOrgs = showOrganizations
-		? (permittedOrgsQuery.data ?? organizations)
+		? (permittedOrgsQuery.data ?? [])
 		: organizations;
 	// Treat the dashboard org as provisional until permissions resolve so
 	// sends and persisted attachments cannot use an unpermitted org.
 	const orgSelectionSettled =
 		!showOrganizations || permittedOrgsQuery.data !== undefined;
-	// Prevent effectiveOrg's dashboard fallback from bypassing an empty
-	// permitted set.
+	// Keep an authoritative empty permission set distinct from pending data.
 	const noPermittedOrgs =
 		showOrganizations && permittedOrgsQuery.data?.length === 0;
 	const selectedOrgIsPermitted =
@@ -228,7 +237,6 @@ export const AgentCreateForm: FC<AgentCreateFormProps> = ({
 			? selectedOrg
 			: (permittedOrgs.find((org) => org.is_default) ??
 				permittedOrgs[0] ??
-				initialOrg ??
 				null);
 	const organizationId = effectiveOrg?.id ?? "";
 	const mcpServersQuery = useQuery({
@@ -266,6 +274,16 @@ export const AgentCreateForm: FC<AgentCreateFormProps> = ({
 			setUserMCPServerIds(null);
 		}
 	}
+	useEffect(() => {
+		if (!orgSelectionSettled) {
+			return;
+		}
+		if (selectedOrg) {
+			localStorage.setItem(selectedOrganizationIdStorageKey, selectedOrg.id);
+		} else {
+			localStorage.removeItem(selectedOrganizationIdStorageKey);
+		}
+	}, [orgSelectionSettled, selectedOrg]);
 	useEffect(() => {
 		if (selectedWorkspaceId === null) {
 			localStorage.removeItem(selectedWorkspaceIdStorageKey);
@@ -453,10 +471,11 @@ export const AgentCreateForm: FC<AgentCreateFormProps> = ({
 	// guarantees completeness. If workspace counts grow large
 	// enough to warrant pagination, this should switch to a
 	// server-side organization:<name> query filter.
-	const filteredWorkspaces =
-		showOrganizations && effectiveOrg
+	const filteredWorkspaces = showOrganizations
+		? effectiveOrg
 			? workspaceOptions.filter((ws) => ws.organization_id === effectiveOrg.id)
-			: workspaceOptions;
+			: []
+		: workspaceOptions;
 
 	const effectiveWorkspaceId =
 		selectedWorkspaceId !== null &&
@@ -609,8 +628,6 @@ export const AgentCreateForm: FC<AgentCreateFormProps> = ({
 								</AlertDescription>
 							</Alert>
 						)}
-					{/* The pre-settlement list is the unfiltered dashboard fallback;
-					    selecting from it could destroy existing workspace state. */}
 					{showOrganizations &&
 						orgSelectionSettled &&
 						permittedOrgs.length > 1 && (