Fix: Don't refresh github token on local (#5880)

All-Hands-AI · Jan 14, 2025 · 580d7b9 · 580d7b9
1 parent 28178a2
commit 580d7b9
Show file tree

Hide file tree

Showing 7 changed files with 75 additions and 36 deletions.
diff --git a/frontend/__tests__/components/browser.test.tsx b/frontend/__tests__/components/browser.test.tsx
@@ -1,11 +1,10 @@
 import { describe, it, expect, afterEach, vi } from "vitest";
-import * as router from "react-router";
 
 // Mock useParams before importing components
 vi.mock("react-router", async () => {
   const actual = await vi.importActual("react-router");
   return {
-    ...actual as object,
+    ...(actual as object),
     useParams: () => ({ conversationId: "test-conversation-id" }),
   };
 });
@@ -14,7 +13,7 @@ vi.mock("react-router", async () => {
 vi.mock("react-i18next", async () => {
   const actual = await vi.importActual("react-i18next");
   return {
-    ...actual as object,
+    ...(actual as object),
     useTranslation: () => ({
       t: (key: string) => key,
       i18n: {
@@ -28,7 +27,6 @@ import { screen } from "@testing-library/react";
 import { renderWithProviders } from "../../test-utils";
 import { BrowserPanel } from "#/components/features/browser/browser";
 
-
 describe("Browser", () => {
   afterEach(() => {
     vi.clearAllMocks();

diff --git a/frontend/__tests__/components/chat/expandable-message.test.tsx b/frontend/__tests__/components/chat/expandable-message.test.tsx
@@ -2,36 +2,42 @@ import { describe, expect, it } from "vitest";
 import { screen } from "@testing-library/react";
 import { renderWithProviders } from "test-utils";
 import { ExpandableMessage } from "#/components/features/chat/expandable-message";
-import { vi } from 'vitest';
+import { vi } from "vitest";
 
-vi.mock('react-i18next', async () => {
-  const actual = await vi.importActual('react-i18next');
+vi.mock("react-i18next", async () => {
+  const actual = await vi.importActual("react-i18next");
   return {
     ...actual,
     useTranslation: () => ({
-      t: (key:string) => key,
+      t: (key: string) => key,
       i18n: {
         changeLanguage: () => new Promise(() => {}),
-        language: 'en',
+        language: "en",
         exists: () => true,
       },
     }),
-  }
+  };
 });
 
 describe("ExpandableMessage", () => {
   it("should render with neutral border for non-action messages", () => {
     renderWithProviders(<ExpandableMessage message="Hello" type="thought" />);
     const element = screen.getByText("Hello");
-    const container = element.closest("div.flex.gap-2.items-center.justify-start");
+    const container = element.closest(
+      "div.flex.gap-2.items-center.justify-start",
+    );
     expect(container).toHaveClass("border-neutral-300");
     expect(screen.queryByTestId("status-icon")).not.toBeInTheDocument();
   });
 
   it("should render with neutral border for error messages", () => {
-    renderWithProviders(<ExpandableMessage message="Error occurred" type="error" />);
+    renderWithProviders(
+      <ExpandableMessage message="Error occurred" type="error" />,
+    );
     const element = screen.getByText("Error occurred");
-    const container = element.closest("div.flex.gap-2.items-center.justify-start");
+    const container = element.closest(
+      "div.flex.gap-2.items-center.justify-start",
+    );
     expect(container).toHaveClass("border-danger");
     expect(screen.queryByTestId("status-icon")).not.toBeInTheDocument();
   });
@@ -43,10 +49,12 @@ describe("ExpandableMessage", () => {
         message="Command executed successfully"
         type="action"
         success={true}
-      />
+      />,
     );
     const element = screen.getByText("OBSERVATION_MESSAGE$RUN");
-    const container = element.closest("div.flex.gap-2.items-center.justify-start");
+    const container = element.closest(
+      "div.flex.gap-2.items-center.justify-start",
+    );
     expect(container).toHaveClass("border-neutral-300");
     const icon = screen.getByTestId("status-icon");
     expect(icon).toHaveClass("fill-success");
@@ -59,10 +67,12 @@ describe("ExpandableMessage", () => {
         message="Command failed"
         type="action"
         success={false}
-      />
+      />,
     );
     const element = screen.getByText("OBSERVATION_MESSAGE$RUN");
-    const container = element.closest("div.flex.gap-2.items-center.justify-start");
+    const container = element.closest(
+      "div.flex.gap-2.items-center.justify-start",
+    );
     expect(container).toHaveClass("border-neutral-300");
     const icon = screen.getByTestId("status-icon");
     expect(icon).toHaveClass("fill-danger");
@@ -74,10 +84,12 @@ describe("ExpandableMessage", () => {
         id="OBSERVATION_MESSAGE$RUN"
         message="Running command"
         type="action"
-      />
+      />,
     );
     const element = screen.getByText("OBSERVATION_MESSAGE$RUN");
-    const container = element.closest("div.flex.gap-2.items-center.justify-start");
+    const container = element.closest(
+      "div.flex.gap-2.items-center.justify-start",
+    );
     expect(container).toHaveClass("border-neutral-300");
     expect(screen.queryByTestId("status-icon")).not.toBeInTheDocument();
   });

diff --git a/frontend/__tests__/components/feedback-form.test.tsx b/frontend/__tests__/components/feedback-form.test.tsx
@@ -1,11 +1,10 @@
-import { afterEach, beforeEach, describe, expect, it, vi } from "vitest";
-import * as router from "react-router";
+import { afterEach, describe, expect, it, vi } from "vitest";
 
 // Mock useParams before importing components
 vi.mock("react-router", async () => {
   const actual = await vi.importActual("react-router");
   return {
-    ...actual as object,
+    ...(actual as object),
     useParams: () => ({ conversationId: "test-conversation-id" }),
   };
 });
@@ -60,7 +59,9 @@ describe("FeedbackForm", () => {
     renderWithProviders(
       <FeedbackForm polarity="positive" onClose={onCloseMock} />,
     );
-    await user.click(screen.getByRole("button", { name: I18nKey.FEEDBACK$CANCEL_LABEL }));
+    await user.click(
+      screen.getByRole("button", { name: I18nKey.FEEDBACK$CANCEL_LABEL }),
+    );
 
     expect(onCloseMock).toHaveBeenCalled();
   });

diff --git a/frontend/__tests__/routes/_oh.test.tsx b/frontend/__tests__/routes/_oh.test.tsx
@@ -1,5 +1,4 @@
 import { afterEach, beforeAll, describe, expect, it, vi } from "vitest";
-import * as router from "react-router";
 import { createRoutesStub } from "react-router";
 import { screen, waitFor, within } from "@testing-library/react";
 import { renderWithProviders } from "test-utils";

diff --git a/frontend/__tests__/utils/test-config.tsx b/frontend/__tests__/utils/test-config.tsx
@@ -0,0 +1,20 @@
+import { vi } from "vitest";
+import OpenHands from "#/api/open-hands";
+
+export const setupTestConfig = () => {
+  const getConfigSpy = vi.spyOn(OpenHands, "getConfig");
+  getConfigSpy.mockResolvedValue({
+    APP_MODE: "oss",
+    GITHUB_CLIENT_ID: "test-id",
+    POSTHOG_CLIENT_KEY: "test-key",
+  });
+};
+
+export const setupSaasTestConfig = () => {
+  const getConfigSpy = vi.spyOn(OpenHands, "getConfig");
+  getConfigSpy.mockResolvedValue({
+    APP_MODE: "saas",
+    GITHUB_CLIENT_ID: "test-id",
+    POSTHOG_CLIENT_KEY: "test-key",
+  });
+};
diff --git a/frontend/src/api/github-axios-instance.ts b/frontend/src/api/github-axios-instance.ts
@@ -41,6 +41,7 @@ export const isGitHubErrorReponse = <T extends object | Array<unknown>>(
 
 // Axios interceptor to handle token refresh
 const setupAxiosInterceptors = (
+  appMode: string,
   refreshToken: () => Promise<boolean>,
   logout: () => void,
 ) => {
@@ -74,18 +75,21 @@ const setupAxiosInterceptors = (
         !originalRequest._retry // Prevent infinite retry loops
       ) {
         originalRequest._retry = true;
-        try {
-          const refreshed = await refreshToken();
-          if (refreshed) {
-            return await github(originalRequest);
-          }
 
-          logout();
-          return await Promise.reject(new Error("Failed to refresh token"));
-        } catch (refreshError) {
-          // If token refresh fails, evict the user
-          logout();
-          return Promise.reject(refreshError);
+        if (appMode === "saas") {
+          try {
+            const refreshed = await refreshToken();
+            if (refreshed) {
+              return await github(originalRequest);
+            }
+
+            logout();
+            return await Promise.reject(new Error("Failed to refresh token"));
+          } catch (refreshError) {
+            // If token refresh fails, evict the user
+            logout();
+            return Promise.reject(refreshError);
+          }
         }
       }
 

diff --git a/frontend/src/context/auth-context.tsx b/frontend/src/context/auth-context.tsx
@@ -87,7 +87,12 @@ function AuthProvider({ children }: React.PropsWithChildren) {
 
     setGitHubToken(storedGitHubToken);
     setUserId(userId);
-    setupGithubAxiosInterceptors(refreshToken, logout);
+    const setupIntercepter = async () => {
+      const config = await OpenHands.getConfig();
+      setupGithubAxiosInterceptors(config.APP_MODE, refreshToken, logout);
+    };
+
+    setupIntercepter();
   }, []);
 
   const value = React.useMemo(