From 2468708293dae020990ce61957cc382149d129af Mon Sep 17 00:00:00 2001 From: Graham Neubig Date: Mon, 27 Apr 2026 12:59:51 -0400 Subject: [PATCH] fix: restore local integration token removal in OSS settings (#14155) Co-authored-by: openhands Co-authored-by: allhands-bot --- AGENTS.md | 1 + .../use-delete-git-providers.test.tsx | 48 +++++++++++++++++++ .../__tests__/routes/git-settings.test.tsx | 8 ++-- frontend/src/api/secrets-service.ts | 5 ++ .../mutation/use-delete-git-providers.ts | 20 ++++++++ frontend/src/routes/git-settings.tsx | 28 +++++++---- 6 files changed, 97 insertions(+), 13 deletions(-) create mode 100644 frontend/__tests__/hooks/mutation/use-delete-git-providers.test.tsx create mode 100644 frontend/src/hooks/mutation/use-delete-git-providers.ts diff --git a/AGENTS.md b/AGENTS.md index 3fca76e185..1d2675dbb2 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -389,6 +389,7 @@ There are two main patterns for saving settings in the OpenHands frontend: **When to use each pattern:** - Use Pattern 1 (Immediate Save) for entity management where each item is independent - Use Pattern 2 (Manual Save) for configuration forms where settings are interdependent or need validation +- Git provider tokens in the local/OSS integrations settings are managed through the V1 secrets endpoints (`POST`/`DELETE /api/v1/secrets/git-providers`). Do not reuse the logout flow for disconnecting tokens; `useLogout` is for actual app logout and still targets legacy OSS logout behavior. ### Adding New LLM Models diff --git a/frontend/__tests__/hooks/mutation/use-delete-git-providers.test.tsx b/frontend/__tests__/hooks/mutation/use-delete-git-providers.test.tsx new file mode 100644 index 0000000000..4f65fb6c20 --- /dev/null +++ b/frontend/__tests__/hooks/mutation/use-delete-git-providers.test.tsx @@ -0,0 +1,48 @@ +import { QueryClient, QueryClientProvider } from "@tanstack/react-query"; +import { renderHook } from "@testing-library/react"; +import React from "react"; +import { beforeEach, describe, expect, it, vi } from "vitest"; +import { SecretsService } from "#/api/secrets-service"; +import { useDeleteGitProviders } from "#/hooks/mutation/use-delete-git-providers"; + +vi.mock("#/context/use-selected-organization", () => ({ + useSelectedOrganizationId: () => ({ organizationId: "org-1" }), +})); + +describe("useDeleteGitProviders", () => { + let queryClient: QueryClient; + + beforeEach(() => { + vi.restoreAllMocks(); + queryClient = new QueryClient({ + defaultOptions: { + queries: { retry: false }, + mutations: { retry: false }, + }, + }); + }); + + it("invalidates personal settings queries after deleting providers", async () => { + vi.spyOn(SecretsService, "deleteGitProviders").mockResolvedValue(true); + + const personalSettingsQueryKey = ["settings", "personal", "org-1"] as const; + queryClient.setQueryData(personalSettingsQueryKey, { + provider_tokens_set: { github: null }, + }); + + const invalidateSpy = vi.spyOn(queryClient, "invalidateQueries"); + + const wrapper = ({ children }: { children: React.ReactNode }) => ( + {children} + ); + + const { result } = renderHook(() => useDeleteGitProviders(), { wrapper }); + + await result.current.mutateAsync(); + + expect(invalidateSpy).toHaveBeenCalled(); + expect(queryClient.getQueryState(personalSettingsQueryKey)?.isInvalidated).toBe( + true, + ); + }); +}); diff --git a/frontend/__tests__/routes/git-settings.test.tsx b/frontend/__tests__/routes/git-settings.test.tsx index 0097223505..96b821e770 100644 --- a/frontend/__tests__/routes/git-settings.test.tsx +++ b/frontend/__tests__/routes/git-settings.test.tsx @@ -8,7 +8,6 @@ import { I18nextProvider } from "react-i18next"; import GitSettingsScreen, { clientLoader } from "#/routes/git-settings"; import SettingsService from "#/api/settings-service/settings-service.api"; import OptionService from "#/api/option-service/option-service.api"; -import AuthService from "#/api/auth-service/auth-service.api"; import { MOCK_DEFAULT_USER_SETTINGS } from "#/mocks/handlers"; import { WebClientConfig } from "#/api/option-service/option.types"; import * as ToastHandlers from "#/utils/custom-toast-handlers"; @@ -484,11 +483,12 @@ describe("Form submission", () => { await waitFor(() => expect(disconnectButton).toBeDisabled()); }); - it("should call logout when pressing the disconnect tokens button", async () => { + it("should delete git providers when pressing the disconnect tokens button", async () => { const getConfigSpy = vi.spyOn(OptionService, "getConfig"); - const logoutSpy = vi.spyOn(AuthService, "logout"); + const deleteGitProvidersSpy = vi.spyOn(SecretsService, "deleteGitProviders"); const getSettingsSpy = vi.spyOn(SettingsService, "getSettings"); + deleteGitProvidersSpy.mockResolvedValue(true); getConfigSpy.mockResolvedValue(VALID_OSS_CONFIG); getSettingsSpy.mockResolvedValue({ ...MOCK_DEFAULT_USER_SETTINGS, @@ -506,7 +506,7 @@ describe("Form submission", () => { await waitFor(() => expect(disconnectButton).not.toBeDisabled()); await userEvent.click(disconnectButton); - expect(logoutSpy).toHaveBeenCalled(); + expect(deleteGitProvidersSpy).toHaveBeenCalled(); }); // flaky test diff --git a/frontend/src/api/secrets-service.ts b/frontend/src/api/secrets-service.ts index dbd755b77c..e9ed037741 100644 --- a/frontend/src/api/secrets-service.ts +++ b/frontend/src/api/secrets-service.ts @@ -94,4 +94,9 @@ export class SecretsService { ); return data; } + + static async deleteGitProviders() { + const { status } = await openHands.delete("/api/v1/secrets/git-providers"); + return status === 200; + } } diff --git a/frontend/src/hooks/mutation/use-delete-git-providers.ts b/frontend/src/hooks/mutation/use-delete-git-providers.ts new file mode 100644 index 0000000000..3188d809e8 --- /dev/null +++ b/frontend/src/hooks/mutation/use-delete-git-providers.ts @@ -0,0 +1,20 @@ +import { useMutation, useQueryClient } from "@tanstack/react-query"; +import { useSelectedOrganizationId } from "#/context/use-selected-organization"; +import { SecretsService } from "#/api/secrets-service"; + +export const useDeleteGitProviders = () => { + const queryClient = useQueryClient(); + const { organizationId } = useSelectedOrganizationId(); + + return useMutation({ + mutationFn: () => SecretsService.deleteGitProviders(), + onSuccess: async () => { + await queryClient.invalidateQueries({ + queryKey: ["settings", "personal", organizationId], + }); + }, + meta: { + disableToast: true, + }, + }); +}; diff --git a/frontend/src/routes/git-settings.tsx b/frontend/src/routes/git-settings.tsx index 53212f647f..526bb0fa11 100644 --- a/frontend/src/routes/git-settings.tsx +++ b/frontend/src/routes/git-settings.tsx @@ -4,7 +4,7 @@ import { useConfig } from "#/hooks/query/use-config"; import { createPermissionGuard } from "#/utils/org/permission-guard"; import { useSettings } from "#/hooks/query/use-settings"; import { BrandButton } from "#/components/features/settings/brand-button"; -import { useLogout } from "#/hooks/mutation/use-logout"; +import { useDeleteGitProviders } from "#/hooks/mutation/use-delete-git-providers"; import { GitHubTokenInput } from "#/components/features/settings/git-settings/github-token-input"; import { GitLabTokenInput } from "#/components/features/settings/git-settings/gitlab-token-input"; import { GitLabWebhookManager } from "#/components/features/settings/git-settings/gitlab-webhook-manager"; @@ -33,7 +33,8 @@ function GitSettingsScreen() { const { t } = useTranslation(); const { mutate: saveGitProviders, isPending } = useAddGitProviders(); - const { mutate: disconnectGitTokens } = useLogout(); + const { mutate: disconnectGitTokens, isPending: isDisconnecting } = + useDeleteGitProviders(); const { data: settings, isLoading } = useSettings(); const { providers } = useUserProviders(); @@ -87,7 +88,15 @@ function GitSettingsScreen() { formData.get("disconnect-tokens-button") !== null; if (disconnectButtonClicked) { - disconnectGitTokens(); + disconnectGitTokens(undefined, { + onSuccess: () => { + displaySuccessToast(t(I18nKey.SETTINGS$SAVED)); + }, + onError: (error) => { + const errorMessage = retrieveAxiosErrorMessage(error); + displayErrorToast(errorMessage || t(I18nKey.ERROR$GENERIC)); + }, + }); return; } @@ -355,12 +364,13 @@ function GitSettingsScreen() { type="submit" variant="secondary" isDisabled={ - !isGitHubTokenSet && - !isGitLabTokenSet && - !isBitbucketTokenSet && - !isBitbucketDCTokenSet && - !isAzureDevOpsTokenSet && - !isForgejoTokenSet + isDisconnecting || + (!isGitHubTokenSet && + !isGitLabTokenSet && + !isBitbucketTokenSet && + !isBitbucketDCTokenSet && + !isAzureDevOpsTokenSet && + !isForgejoTokenSet) } > {t(I18nKey.GIT$DISCONNECT_TOKENS)}