diff --git a/frontend/__tests__/components/features/settings/org-llm-profiles-manager.test.tsx b/frontend/__tests__/components/features/settings/org-llm-profiles-manager.test.tsx new file mode 100644 index 0000000000..82e08ac730 --- /dev/null +++ b/frontend/__tests__/components/features/settings/org-llm-profiles-manager.test.tsx @@ -0,0 +1,112 @@ +import { render, screen } from "@testing-library/react"; +import userEvent from "@testing-library/user-event"; +import { beforeEach, describe, expect, it, vi } from "vitest"; +import type { LlmProfileSummary } from "#/api/settings-service/profiles-service.api"; +import { OrgLlmProfilesManager } from "#/components/features/settings/org-llm-profiles-manager"; + +type ProfilesList = { + profiles: LlmProfileSummary[]; + active_profile: string | null; +}; + +const profilesState: { + data: ProfilesList | undefined; + isLoading: boolean; + error: Error | null; +} = { data: undefined, isLoading: false, error: null }; + +const activateMock = vi.fn(); +const deleteMock = vi.fn(); +const renameMock = vi.fn(); + +vi.mock("#/hooks/query/use-org-llm-profiles", () => ({ + useOrgLlmProfiles: () => profilesState, +})); + +vi.mock("#/hooks/mutation/use-org-llm-profile-mutations", () => ({ + useActivateOrgLlmProfile: () => ({ + mutateAsync: activateMock, + isPending: false, + }), + useDeleteOrgLlmProfile: () => ({ + mutateAsync: deleteMock, + isPending: false, + }), + useRenameOrgLlmProfile: () => ({ + mutateAsync: renameMock, + isPending: false, + }), +})); + +const sampleProfiles: ProfilesList = { + profiles: [ + { + name: "sonnet", + model: "openhands/claude-sonnet-4-5-20250929", + base_url: null, + api_key_set: true, + }, + { + name: "opus", + model: "openhands/claude-opus-4-7", + base_url: null, + api_key_set: true, + }, + ], + active_profile: "sonnet", +}; + +function renderManager({ + canManage, + onAddProfile, + onEditProfile, +}: { + canManage?: boolean; + onAddProfile?: () => void; + onEditProfile?: (profile: LlmProfileSummary) => void; +} = {}) { + return render( + , + ); +} + +beforeEach(() => { + profilesState.data = sampleProfiles; + profilesState.isLoading = false; + profilesState.error = null; + activateMock.mockReset().mockResolvedValue(undefined); + deleteMock.mockReset().mockResolvedValue(undefined); + renameMock.mockReset().mockResolvedValue(undefined); +}); + +describe("OrgLlmProfilesManager", () => { + it("renders Add LLM Profile when management is enabled", async () => { + const onAddProfile = vi.fn(); + renderManager({ canManage: true, onAddProfile }); + const user = userEvent.setup(); + + await user.click(screen.getByTestId("add-llm-profile")); + + expect(onAddProfile).toHaveBeenCalledTimes(1); + }); + + it("renders profiles without management controls when management is disabled", () => { + renderManager({ + canManage: false, + onAddProfile: vi.fn(), + onEditProfile: vi.fn(), + }); + + expect(screen.getByText("sonnet")).toBeInTheDocument(); + expect(screen.getByText("opus")).toBeInTheDocument(); + expect(screen.queryByTestId("add-llm-profile")).not.toBeInTheDocument(); + expect( + screen.queryByTestId("profile-menu-trigger"), + ).not.toBeInTheDocument(); + }); +}); diff --git a/frontend/__tests__/components/features/settings/profile-actions-menu.test.tsx b/frontend/__tests__/components/features/settings/profile-actions-menu.test.tsx index cda8e14bcf..fb207e74c2 100644 --- a/frontend/__tests__/components/features/settings/profile-actions-menu.test.tsx +++ b/frontend/__tests__/components/features/settings/profile-actions-menu.test.tsx @@ -42,13 +42,13 @@ describe("ProfileActionsMenu", () => { expect(props.onClose).toHaveBeenCalledTimes(1); }); - it("disables Set as active when the profile is already active", () => { + it("disables Set as default when the profile is already default", () => { renderMenu({ isActive: true }); expect(screen.getByTestId("profile-set-active")).toBeDisabled(); }); - it("disables Set as active while an activation is in flight", () => { + it("disables Set as default while an activation is in flight", () => { renderMenu({ isActivating: true }); expect(screen.getByTestId("profile-set-active")).toBeDisabled(); diff --git a/frontend/__tests__/components/features/settings/profile-row.test.tsx b/frontend/__tests__/components/features/settings/profile-row.test.tsx index 781ef0e422..08f30f67f1 100644 --- a/frontend/__tests__/components/features/settings/profile-row.test.tsx +++ b/frontend/__tests__/components/features/settings/profile-row.test.tsx @@ -58,4 +58,13 @@ describe("ProfileRow", () => { await user.click(screen.getByTestId("profile-menu-trigger")); expect(screen.getByTestId("profile-edit")).toBeInTheDocument(); }); + + it("hides the actions menu trigger when management is disabled", () => { + renderRow({ canManage: false }); + + expect(screen.getByText("openai_gpt-4o")).toBeInTheDocument(); + expect( + screen.queryByTestId("profile-menu-trigger"), + ).not.toBeInTheDocument(); + }); }); diff --git a/frontend/__tests__/components/features/settings/profiles-body.test.tsx b/frontend/__tests__/components/features/settings/profiles-body.test.tsx index 6487522cf6..96ca82dc05 100644 --- a/frontend/__tests__/components/features/settings/profiles-body.test.tsx +++ b/frontend/__tests__/components/features/settings/profiles-body.test.tsx @@ -86,4 +86,26 @@ describe("ProfilesBody", () => { expect(rows[0]).toHaveTextContent("SETTINGS$PROFILE_ACTIVE_BADGE"); expect(rows[1]).not.toHaveTextContent("SETTINGS$PROFILE_ACTIVE_BADGE"); }); + + it("renders profiles without action menus when management is disabled", () => { + render( + , + ); + + expect(screen.getAllByTestId("profile-row")).toHaveLength(2); + expect( + screen.queryByTestId("profile-menu-trigger"), + ).not.toBeInTheDocument(); + }); }); diff --git a/frontend/__tests__/routes/llm-settings.test.tsx b/frontend/__tests__/routes/llm-settings.test.tsx index 4e232f8fa0..c43fce2d5c 100644 --- a/frontend/__tests__/routes/llm-settings.test.tsx +++ b/frontend/__tests__/routes/llm-settings.test.tsx @@ -4,6 +4,7 @@ import userEvent from "@testing-library/user-event"; import { beforeEach, describe, expect, it, vi } from "vitest"; import { MemoryRouter } from "react-router"; +import OrgProfilesService from "#/api/organization-service/org-profiles-service.api"; import { organizationService } from "#/api/organization-service/organization-service.api"; import ProfilesService from "#/api/settings-service/profiles-service.api"; import SettingsService from "#/api/settings-service/settings-service.api"; @@ -30,6 +31,17 @@ vi.mock("#/api/settings-service/profiles-service.api", () => ({ }, })); +vi.mock("#/api/organization-service/org-profiles-service.api", () => ({ + default: { + listProfiles: vi.fn(), + getProfile: vi.fn(), + saveProfile: vi.fn(), + deleteProfile: vi.fn(), + activateProfile: vi.fn(), + renameProfile: vi.fn(), + }, +})); + function resetProfilesServiceDefaults() { vi.mocked(ProfilesService.listProfiles) .mockReset() @@ -48,6 +60,30 @@ function resetProfilesServiceDefaults() { .mockResolvedValue(undefined); } +function resetOrgProfilesServiceDefaults() { + vi.mocked(OrgProfilesService.listProfiles) + .mockReset() + .mockResolvedValue({ profiles: [], active_profile: null }); + vi.mocked(OrgProfilesService.getProfile) + .mockReset() + .mockResolvedValue({ + name: "openai_gpt-4o", + llm: { model: "openai/gpt-4o" }, + }); + vi.mocked(OrgProfilesService.saveProfile) + .mockReset() + .mockResolvedValue(undefined); + vi.mocked(OrgProfilesService.deleteProfile) + .mockReset() + .mockResolvedValue(undefined); + vi.mocked(OrgProfilesService.activateProfile) + .mockReset() + .mockResolvedValue(undefined); + vi.mocked(OrgProfilesService.renameProfile) + .mockReset() + .mockResolvedValue(undefined); +} + // Stub the profile mutation hooks so auto-save doesn't invalidate the // settings query — tests in this file pin exact getSettings call counts. // The mutateAsync implementations forward to the service mock so the @@ -215,7 +251,7 @@ async function renderLlmSettingsScreen({ meData?: OrganizationMember; organizations?: Organization[]; scope?: "personal" | "org"; - // Personal scope now lands on the Available Models list by default; set + // Profile-enabled scopes land on the Available Models list by default; set // ``view`` to ``"form"`` (the default) to auto-click into the SDK form // so existing form-oriented assertions keep working unchanged, or to // ``"profiles"`` to test the list view itself. @@ -250,12 +286,14 @@ async function renderLlmSettingsScreen({ const rendered = render(, { wrapper: ({ children }) => ( - {children} + + {children} + ), }); - if (scope === "personal" && view === "form") { + if (view === "form") { await userEvent.click(await screen.findByTestId("add-llm-profile")); } @@ -265,6 +303,7 @@ async function renderLlmSettingsScreen({ beforeEach(() => { vi.restoreAllMocks(); resetProfilesServiceDefaults(); + resetOrgProfilesServiceDefaults(); resetTestHandlersMockSettings(); mockUseSearchParams.mockReturnValue([{ get: () => null }, vi.fn()]); mockUseConfig.mockReturnValue({ @@ -312,7 +351,7 @@ describe("LlmSettingsScreen", () => { it("shows Advanced and All toggles in OSS mode for the default LLM route schema", async () => { vi.spyOn(SettingsService, "getSettings").mockResolvedValue(buildSettings()); - renderLlmSettingsScreen({ appMode: "oss" }); + await renderLlmSettingsScreen({ appMode: "oss" }); await screen.findByTestId("llm-settings-screen"); expect( @@ -332,7 +371,7 @@ describe("LlmSettingsScreen", () => { }), ); - renderLlmSettingsScreen({ appMode: "saas", scope: "org" }); + await renderLlmSettingsScreen({ appMode: "saas", scope: "org" }); await screen.findByTestId("llm-settings-screen"); expect( @@ -914,7 +953,7 @@ describe("LlmSettingsScreen", () => { return true; }); - renderLlmSettingsScreen({ appMode: "oss" }); + await renderLlmSettingsScreen({ appMode: "oss" }); await screen.findByTestId("llm-settings-form-basic"); @@ -1196,9 +1235,20 @@ describe("LlmSettingsScreen", () => { expect(payload.settings).not.toHaveProperty("search_api_key"); await waitFor(() => { - expect(getOrganizationSettingsSpy).toHaveBeenCalledTimes(2); + expect(getOrganizationSettingsSpy).toHaveBeenCalledTimes(3); }); + await waitFor(() => { + expect(screen.getByTestId("add-llm-profile")).toBeInTheDocument(); + expect( + screen.queryByTestId("llm-settings-form-basic"), + ).not.toBeInTheDocument(); + expect( + screen.queryByTestId("llm-settings-form-advanced"), + ).not.toBeInTheDocument(); + }); + + await userEvent.click(screen.getByTestId("add-llm-profile")); await waitFor(() => { expect(screen.getByTestId("llm-settings-form-basic")).toBeInTheDocument(); expect( @@ -1767,6 +1817,62 @@ describe("LlmSettingsScreen", () => { }); describe("Role-based permissions", () => { + describe("Org default profiles", () => { + it("shows org profiles and management controls for admins", async () => { + vi.mocked(OrgProfilesService.listProfiles).mockResolvedValue({ + profiles: [ + { + name: "sonnet", + model: "openhands/claude-sonnet-4-5-20250929", + base_url: null, + api_key_set: true, + }, + ], + active_profile: "sonnet", + }); + + await renderLlmSettingsScreen({ + appMode: "saas", + scope: "org", + organizationId: "3", + meData: buildOrganizationMember({ org_id: "3", role: "admin" }), + view: "profiles", + }); + + expect(await screen.findByText("sonnet")).toBeInTheDocument(); + expect(screen.getByTestId("add-llm-profile")).toBeInTheDocument(); + expect(screen.getByTestId("profile-menu-trigger")).toBeInTheDocument(); + }); + + it("shows org profiles without management controls for members", async () => { + vi.mocked(OrgProfilesService.listProfiles).mockResolvedValue({ + profiles: [ + { + name: "sonnet", + model: "openhands/claude-sonnet-4-5-20250929", + base_url: null, + api_key_set: true, + }, + ], + active_profile: "sonnet", + }); + + await renderLlmSettingsScreen({ + appMode: "saas", + scope: "org", + organizationId: "2", + meData: buildOrganizationMember({ org_id: "2", role: "member" }), + view: "profiles", + }); + + expect(await screen.findByText("sonnet")).toBeInTheDocument(); + expect(screen.queryByTestId("add-llm-profile")).not.toBeInTheDocument(); + expect( + screen.queryByTestId("profile-menu-trigger"), + ).not.toBeInTheDocument(); + }); + }); + describe("Member role (personal overrides allowed)", () => { it("should keep all input fields enabled in basic view", async () => { vi.spyOn(SettingsService, "getSettings").mockResolvedValue( @@ -2135,10 +2241,7 @@ describe("LlmSettingsScreen", () => { }); }); - it("does NOT auto-save a profile on the org-default settings screen", async () => { - // Org defaults reuse this screen with scope="org". Profiles are a - // per-user feature, so touching the profiles endpoints here would - // incorrectly spawn profiles on the signed-in user's settings. + it("saves + activates an org profile on the org-default settings screen", async () => { vi.spyOn( organizationService, "getOrganizationSettings", @@ -2164,6 +2267,10 @@ describe("LlmSettingsScreen", () => { meData: buildOrganizationMember({ org_id: "3", role: "admin" }), }); + await userEvent.type( + await screen.findByTestId("llm-profile-name-input"), + "team-profile", + ); await userEvent.type( await screen.findByTestId("llm-api-key-input"), "test-api-key", @@ -2173,6 +2280,19 @@ describe("LlmSettingsScreen", () => { await waitFor(() => { expect(organizationService.saveOrganizationSettings).toHaveBeenCalled(); }); + await waitFor(() => { + expect(OrgProfilesService.saveProfile).toHaveBeenCalledWith( + "3", + "team-profile", + { include_secrets: true }, + ); + }); + await waitFor(() => { + expect(OrgProfilesService.activateProfile).toHaveBeenCalledWith( + "3", + "team-profile", + ); + }); expect(ProfilesService.saveProfile).not.toHaveBeenCalled(); expect(ProfilesService.activateProfile).not.toHaveBeenCalled(); }); @@ -2244,7 +2364,7 @@ describe("LlmSettingsScreen", () => { }); }); - it("does not render the profile-name input on the org-default settings screen", async () => { + it("renders the profile-name input on the org-default profile form for admins", async () => { vi.spyOn( organizationService, "getOrganizationSettings", @@ -2261,12 +2381,8 @@ describe("LlmSettingsScreen", () => { meData: buildOrganizationMember({ org_id: "3", role: "admin" }), }); - // Wait for the form to render (org-defaults takes the same screen - // but should never offer to name a profile). await screen.findByTestId("llm-api-key-input"); - expect( - screen.queryByTestId("llm-profile-name-input"), - ).not.toBeInTheDocument(); + expect(screen.getByTestId("llm-profile-name-input")).toBeInTheDocument(); }); it("swallows profile-save failures so the user still sees the settings-saved toast", async () => { diff --git a/frontend/src/components/features/settings/org-llm-profiles-manager.tsx b/frontend/src/components/features/settings/org-llm-profiles-manager.tsx index d72fe0bc5d..7c739d8eee 100644 --- a/frontend/src/components/features/settings/org-llm-profiles-manager.tsx +++ b/frontend/src/components/features/settings/org-llm-profiles-manager.tsx @@ -13,12 +13,14 @@ import { I18nKey } from "#/i18n/declaration"; interface OrgLlmProfilesManagerProps { orgId: string; + canManage?: boolean; onAddProfile?: () => void; onEditProfile?: (profile: LlmProfileSummary) => void; } export function OrgLlmProfilesManager({ orgId, + canManage = true, onAddProfile, onEditProfile, }: OrgLlmProfilesManagerProps) { @@ -52,7 +54,7 @@ export function OrgLlmProfilesManager({

{t(I18nKey.SETTINGS$AVAILABLE_PROFILES)}

- {onAddProfile ? ( + {canManage && onAddProfile ? ( diff --git a/frontend/src/components/features/settings/profile-row.tsx b/frontend/src/components/features/settings/profile-row.tsx index 8f7cbed13a..3a5624858f 100644 --- a/frontend/src/components/features/settings/profile-row.tsx +++ b/frontend/src/components/features/settings/profile-row.tsx @@ -14,6 +14,7 @@ interface ProfileRowProps { onRename: (profile: LlmProfileSummary) => void; onDelete: (profile: LlmProfileSummary) => void; isActivating: boolean; + canManage?: boolean; } export function ProfileRow({ @@ -24,6 +25,7 @@ export function ProfileRow({ onRename, onDelete, isActivating, + canManage = true, }: ProfileRowProps) { const { t } = useTranslation(); const [menuOpen, setMenuOpen] = useState(false); @@ -57,28 +59,30 @@ export function ProfileRow({ )} -
- - {menuOpen && ( - onEdit(profile)} - onRename={() => onRename(profile)} - onSetActive={() => onActivate(profile.name)} - onDelete={() => onDelete(profile)} - isActive={isActive} - isActivating={isActivating} - onClose={() => setMenuOpen(false)} - /> - )} -
+ {canManage ? ( +
+ + {menuOpen && ( + onEdit(profile)} + onRename={() => onRename(profile)} + onSetActive={() => onActivate(profile.name)} + onDelete={() => onDelete(profile)} + isActive={isActive} + isActivating={isActivating} + onClose={() => setMenuOpen(false)} + /> + )} +
+ ) : null} ); } diff --git a/frontend/src/components/features/settings/profiles-body.tsx b/frontend/src/components/features/settings/profiles-body.tsx index 7a2bd03e9e..b223eeb050 100644 --- a/frontend/src/components/features/settings/profiles-body.tsx +++ b/frontend/src/components/features/settings/profiles-body.tsx @@ -15,6 +15,7 @@ interface ProfilesBodyProps { onRename: (profile: LlmProfileSummary) => void; onDelete: (profile: LlmProfileSummary) => void; isActivating: boolean; + canManage?: boolean; } export function ProfilesBody({ @@ -27,6 +28,7 @@ export function ProfilesBody({ onRename, onDelete, isActivating, + canManage = true, }: ProfilesBodyProps) { const { t } = useTranslation(); @@ -63,6 +65,7 @@ export function ProfilesBody({ onRename={onRename} onDelete={onDelete} isActivating={isActivating} + canManage={canManage} /> ))} diff --git a/frontend/src/i18n/translation.json b/frontend/src/i18n/translation.json index da87dafb87..112d5b1969 100644 --- a/frontend/src/i18n/translation.json +++ b/frontend/src/i18n/translation.json @@ -25347,7 +25347,7 @@ "uk": "Не вдалося завантажити профілі. Спробуйте ще раз." }, "SETTINGS$PROFILE_ACTIVE_BADGE": { - "en": "Active", + "en": "Default", "ja": "アクティブ", "zh-CN": "活动", "zh-TW": "使用中", @@ -25483,7 +25483,7 @@ "uk": "Профіль \"{{name}}\" видалено" }, "SETTINGS$PROFILE_ACTIVATED": { - "en": "Switched to profile \"{{name}}\"", + "en": "Default profile set to \"{{name}}\"", "ja": "プロファイル\"{{name}}\"に切り替えました", "zh-CN": "已切换到配置文件 \"{{name}}\"", "zh-TW": "已切換到設定檔 \"{{name}}\"", @@ -25568,7 +25568,7 @@ "uk": "Редагувати" }, "SETTINGS$PROFILE_SET_ACTIVE": { - "en": "Set as active", + "en": "Set as default", "ja": "アクティブに設定", "zh-CN": "设为活动", "zh-TW": "設為使用中", diff --git a/frontend/src/routes/llm-settings.tsx b/frontend/src/routes/llm-settings.tsx index de4272bfde..7b9074d2cb 100644 --- a/frontend/src/routes/llm-settings.tsx +++ b/frontend/src/routes/llm-settings.tsx @@ -45,6 +45,8 @@ import { OrgLlmProfilesManager } from "#/components/features/settings/org-llm-pr import { ProfileNameInput } from "#/components/features/settings/profile-name-input"; import { Typography } from "#/ui/typography"; import { useOrgTypeAndAccess } from "#/hooks/use-org-type-and-access"; +import { useMe } from "#/hooks/query/use-me"; +import { usePermission } from "#/hooks/organizations/use-permissions"; const LLM_EXCLUDED_KEYS = new Set(["llm.model", "llm.api_key", "llm.base_url"]); @@ -112,7 +114,9 @@ export function LlmSettingsScreen({ settings?.agent_settings_schema, ); const { data: config } = useConfig(); - const { isPersonalOrg, organizationId } = useOrgTypeAndAccess(); + const { organizationId } = useOrgTypeAndAccess(); + const { data: me } = useMe(); + const { hasPermission } = usePermission(me?.role ?? "member"); const [selectedProvider, setSelectedProvider] = React.useState( null, @@ -127,7 +131,7 @@ export function LlmSettingsScreen({ const activateProfile = useActivateLlmProfile(); const renameProfile = useRenameLlmProfile(); - // Org profile hooks (for SaaS mode with personal orgs) + // Org profile hooks (for org defaults) const saveOrgProfile = useSaveOrgLlmProfile(organizationId); const activateOrgProfile = useActivateOrgLlmProfile(organizationId); const renameOrgProfile = useRenameOrgLlmProfile(organizationId); @@ -135,11 +139,11 @@ export function LlmSettingsScreen({ // Controls whether the LLM form or the Profiles list is shown. Flipping // this unmounts the inactive branch, so the SdkSectionPage re-hydrates // its view from ``initialViewHint`` when coming back from profiles. - // Enable profiles for: - // - Personal scope (OSS mode) - // - Org scope with personal org (SaaS mode) - const shouldShowProfilesForScope = - scope === "personal" || (scope === "org" && isPersonalOrg); + // Enable profiles for personal settings and org defaults. Org members can + // view org profiles, but only admins/owners can create or manage them. + const shouldShowProfilesForScope = scope === "personal" || scope === "org"; + const canManageProfilesForScope = + scope === "personal" || hasPermission("edit_llm_settings"); const [showProfiles, setShowProfiles] = React.useState( shouldShowProfilesForScope, ); @@ -158,10 +162,8 @@ export function LlmSettingsScreen({ const [initialViewHint, setInitialViewHint] = React.useState(null); - // Show profiles view for personal scope OR org scope with personal org const isProfilesView = shouldShowProfilesForScope && showProfiles; - // Use org-scoped profile operations when in org scope - const isOrgProfileMode = scope === "org" && isPersonalOrg; + const isOrgProfileMode = scope === "org"; const defaultModel = String( (DEFAULT_SETTINGS.agent_settings?.llm as Record)?.model ?? @@ -306,7 +308,7 @@ export function LlmSettingsScreen({ ) : null} - {shouldShowProfilesForScope ? ( + {canManageProfilesForScope ? ( openForm(null)} - onEditProfile={(profile) => openForm(null, profile.name)} + canManage={canManageProfilesForScope} + onAddProfile={ + canManageProfilesForScope ? () => openForm(null) : undefined + } + onEditProfile={ + canManageProfilesForScope + ? (profile) => openForm(null, profile.name) + : undefined + } /> ); } @@ -558,9 +568,9 @@ export function LlmSettingsScreen({ ); } - // Sub-page back affordance when profiles are enabled (personal scope or - // personal org). Replaces the previous "Profiles" trailing action so the - // form view follows the second-level settings pattern. + // Sub-page back affordance when profiles are enabled. Replaces the previous + // "Profiles" trailing action so the form view follows the second-level + // settings pattern. const backToProfiles = shouldShowProfilesForScope ? (