mirror of
https://github.com/OpenHands/OpenHands.git
synced 2026-10-07 12:58:49 +08:00
fix(BM-001): auto-switch active backend on addBackend (#714)
* spec(BM-001): add spec + failing tests for auto-switch on connect Adding a backend should automatically switch the active selection to it. Currently addBackend registers the entry but leaves the user on the previous backend, forcing a manual switch. - specs/backend-management.md: BM-001 definition - Two new test cases (cloud + local) that assert the active backend changes after addBackend — both fail against the current implementation. Co-authored-by: openhands <openhands@all-hands.dev> * fix(BM-001): auto-switch active backend on addBackend addBackend now calls setActiveSelection after registering the new entry, so the user lands on the backend they just connected — whether via the manual host+key form or the cloud OAuth device-flow login. - src/contexts/active-backend-context.tsx: one-line fix in addBackend - Updated add-backend-modal test to assert the new active selection - Updated backend-selector tests that used addBackend purely as setup to reset active to the default local backend afterward Co-authored-by: openhands <openhands@all-hands.dev> * chore: remove noisy @spec markers from test setup comments Keep @spec BM-001 only where it marks the implementation or directly asserts the spec behavior. Setup-only adjustments (resetting active backend after addBackend) are incidental — plain comments suffice. Co-authored-by: openhands <openhands@all-hands.dev> * chore: consolidate @spec BM-001 to one test + one implementation site The spec marker belongs in exactly two places: the line that implements the behavior and the single test that verifies it. Removed the redundant local-backend variant (same code path as cloud) and dropped @spec labels from the modal test (its assertion stays, just without the tag). Co-authored-by: openhands <openhands@all-hands.dev> * refactor: remove setActive workarounds from backend-selector tests Instead of manually resetting active state after addBackend, tests now work with the auto-switch naturally: - Local backend tests: click the seeded default 'Local' (which is no longer active after auto-switch) to trigger the switch. - Cloud org tests: add a local backend after the cloud one so the last auto-switch lands on local, leaving cloud backends unselected and their org rows visible in the dropdown. No ctx.setActive(DEFAULT_LOCAL_BACKEND_ID) calls remain. Co-authored-by: openhands <openhands@all-hands.dev> * refactor: extract shared seed constants in backend-selector tests SEED_LOCAL_1 and SEED_CLOUD_PRODUCTION replace 12+4 identical inline config objects. The two remaining inline blocks have different apiKey values and correctly stay as-is. Co-authored-by: openhands <openhands@all-hands.dev> --------- Co-authored-by: openhands <openhands@all-hands.dev>
This commit is contained in:
committed by
GitHub
co-authored by
openhands
parent
5a64e0b3c3
commit
e6d12e3fff
@@ -118,7 +118,7 @@ describe("AddBackendModal – two-column layout", () => {
|
||||
expect(submit).not.toBeDisabled();
|
||||
});
|
||||
|
||||
it("saves the backend and closes WITHOUT switching the active selection", async () => {
|
||||
it("saves the backend, switches to it, and closes", async () => {
|
||||
const onClose = vi.fn();
|
||||
renderWithProviders(<AddBackendModal onClose={onClose} />);
|
||||
|
||||
@@ -146,8 +146,11 @@ describe("AddBackendModal – two-column layout", () => {
|
||||
kind: "local",
|
||||
});
|
||||
|
||||
// Adding a backend must NOT change the active selection.
|
||||
expect(window.localStorage.getItem("openhands-active-backend")).toBeNull();
|
||||
// Active selection must point at the newly added backend.
|
||||
const active = JSON.parse(
|
||||
window.localStorage.getItem("openhands-active-backend") ?? "null",
|
||||
);
|
||||
expect(active).toEqual({ backendId: added.id, orgId: null });
|
||||
});
|
||||
|
||||
it("shows the close button", () => {
|
||||
|
||||
@@ -39,6 +39,21 @@ vi.mock("@openhands/typescript-client/clients", () => ({
|
||||
ServerClient: vi.fn(),
|
||||
}));
|
||||
|
||||
// Shared seed configs reused across tests.
|
||||
const SEED_LOCAL_1 = {
|
||||
name: "Local 1",
|
||||
host: "http://localhost:9000",
|
||||
apiKey: "k",
|
||||
kind: "local" as const,
|
||||
};
|
||||
|
||||
const SEED_CLOUD_PRODUCTION = {
|
||||
name: "Production",
|
||||
host: "https://app.all-hands.dev",
|
||||
apiKey: "bearer-key",
|
||||
kind: "cloud" as const,
|
||||
};
|
||||
|
||||
function renderWithProviders(ui: React.ReactElement) {
|
||||
const queryClient = new QueryClient({
|
||||
defaultOptions: { queries: { retry: false } },
|
||||
@@ -141,12 +156,7 @@ describe("BackendSelector", () => {
|
||||
renderWithProviders(
|
||||
<TestSeed
|
||||
onMount={(ctx) => {
|
||||
ctx.addBackend({
|
||||
name: "Local 1",
|
||||
host: "http://localhost:9000",
|
||||
apiKey: "k",
|
||||
kind: "local",
|
||||
});
|
||||
ctx.addBackend(SEED_LOCAL_1);
|
||||
ctx.addBackend({
|
||||
name: "Production",
|
||||
host: "https://app.all-hands.dev",
|
||||
@@ -181,12 +191,11 @@ describe("BackendSelector", () => {
|
||||
renderWithProviders(
|
||||
<TestSeed
|
||||
onMount={(ctx) => {
|
||||
cloudId = ctx.addBackend({
|
||||
name: "Production",
|
||||
host: "https://app.all-hands.dev",
|
||||
apiKey: "bearer-key",
|
||||
kind: "cloud",
|
||||
}).id;
|
||||
cloudId = ctx.addBackend(SEED_CLOUD_PRODUCTION).id;
|
||||
// Add a second backend so auto-switch lands here, leaving the
|
||||
// cloud backend unselected (the dropdown only expands org rows
|
||||
// for non-active cloud backends).
|
||||
ctx.addBackend(SEED_LOCAL_1);
|
||||
}}
|
||||
>
|
||||
<BackendSelector />
|
||||
@@ -245,6 +254,9 @@ describe("BackendSelector", () => {
|
||||
apiKey: "key-acme",
|
||||
kind: "cloud",
|
||||
});
|
||||
// Land on a local backend so both cloud backends are unselected
|
||||
// and their org rows render in the dropdown.
|
||||
ctx.addBackend(SEED_LOCAL_1);
|
||||
}}
|
||||
>
|
||||
<BackendSelector />
|
||||
@@ -289,12 +301,10 @@ describe("BackendSelector", () => {
|
||||
renderWithProviders(
|
||||
<TestSeed
|
||||
onMount={(ctx) => {
|
||||
ctx.addBackend({
|
||||
name: "Production",
|
||||
host: "https://app.all-hands.dev",
|
||||
apiKey: "bearer-key",
|
||||
kind: "cloud",
|
||||
});
|
||||
ctx.addBackend(SEED_CLOUD_PRODUCTION);
|
||||
// Land on a local backend so the cloud backend is unselected
|
||||
// and its org rows render in the dropdown.
|
||||
ctx.addBackend(SEED_LOCAL_1);
|
||||
}}
|
||||
>
|
||||
<BackendSelector />
|
||||
@@ -333,12 +343,7 @@ describe("BackendSelector", () => {
|
||||
renderWithProviders(
|
||||
<TestSeed
|
||||
onMount={(ctx) => {
|
||||
cloudId = ctx.addBackend({
|
||||
name: "Production",
|
||||
host: "https://app.all-hands.dev",
|
||||
apiKey: "bearer-key",
|
||||
kind: "cloud",
|
||||
}).id;
|
||||
cloudId = ctx.addBackend(SEED_CLOUD_PRODUCTION).id;
|
||||
// Simulate the post-refresh malformed state: active backend is
|
||||
// the cloud one but no orgId is set yet.
|
||||
ctx.setActive(cloudId, null);
|
||||
@@ -364,24 +369,20 @@ describe("BackendSelector", () => {
|
||||
renderWithProviders(
|
||||
<TestSeed
|
||||
onMount={(ctx) => {
|
||||
ctx.addBackend({
|
||||
name: "Local 1",
|
||||
host: "http://localhost:9000",
|
||||
apiKey: "k",
|
||||
kind: "local",
|
||||
});
|
||||
ctx.addBackend(SEED_LOCAL_1);
|
||||
}}
|
||||
>
|
||||
<BackendSelector />
|
||||
</TestSeed>,
|
||||
);
|
||||
|
||||
// Auto-switch lands on "Local 1"; click the seeded default to switch.
|
||||
const user = await openDropdown();
|
||||
await user.click(screen.getByText("Local 1"));
|
||||
await user.click(screen.getByText("Local"));
|
||||
|
||||
const wrapper = screen.getByTestId("backend-selector");
|
||||
const input = wrapper.querySelector("input") as HTMLInputElement;
|
||||
expect(input.value).toBe("Local 1");
|
||||
expect(input.value).toBe("Local");
|
||||
});
|
||||
|
||||
it("redirects to home when switching backends from a conversation route", async () => {
|
||||
@@ -389,12 +390,7 @@ describe("BackendSelector", () => {
|
||||
return (
|
||||
<TestSeed
|
||||
onMount={(ctx) => {
|
||||
ctx.addBackend({
|
||||
name: "Local 1",
|
||||
host: "http://localhost:9000",
|
||||
apiKey: "k",
|
||||
kind: "local",
|
||||
});
|
||||
ctx.addBackend(SEED_LOCAL_1);
|
||||
}}
|
||||
>
|
||||
<BackendSelector />
|
||||
@@ -419,8 +415,9 @@ describe("BackendSelector", () => {
|
||||
</QueryClientProvider>,
|
||||
);
|
||||
|
||||
// Auto-switch lands on "Local 1"; click the seeded default to switch.
|
||||
const user = await openDropdown();
|
||||
await user.click(screen.getByText("Local 1"));
|
||||
await user.click(screen.getByText("Local"));
|
||||
|
||||
expect(await screen.findByTestId("home")).toBeInTheDocument();
|
||||
});
|
||||
@@ -430,12 +427,7 @@ describe("BackendSelector", () => {
|
||||
return (
|
||||
<TestSeed
|
||||
onMount={(ctx) => {
|
||||
ctx.addBackend({
|
||||
name: "Local 1",
|
||||
host: "http://localhost:9000",
|
||||
apiKey: "k",
|
||||
kind: "local",
|
||||
});
|
||||
ctx.addBackend(SEED_LOCAL_1);
|
||||
}}
|
||||
>
|
||||
<div data-testid="settings-route" />
|
||||
@@ -461,8 +453,9 @@ describe("BackendSelector", () => {
|
||||
</QueryClientProvider>,
|
||||
);
|
||||
|
||||
// Auto-switch lands on "Local 1"; click the seeded default to switch.
|
||||
const user = await openDropdown();
|
||||
await user.click(screen.getByText("Local 1"));
|
||||
await user.click(screen.getByText("Local"));
|
||||
|
||||
// The settings route is still in the DOM after the switch — no
|
||||
// redirect to /conversations or anywhere else.
|
||||
@@ -495,17 +488,17 @@ describe("BackendSelector", () => {
|
||||
);
|
||||
render(<EnvironmentSwitchOverlay />);
|
||||
|
||||
// Act — select a different backend, then immediately unmount the
|
||||
// selector (the click itself would do this in production via the
|
||||
// outside-click handler).
|
||||
// Act — auto-switch lands on "Acme Local"; click the seeded default
|
||||
// to trigger a switch, then immediately unmount the selector (the
|
||||
// click itself would do this in production via the outside-click handler).
|
||||
const user = await openDropdown();
|
||||
await user.click(screen.getByText("Acme Local"));
|
||||
await user.click(screen.getByText("Local"));
|
||||
selectorRender.unmount();
|
||||
|
||||
// Assert — the overlay is still in the DOM with the chosen target
|
||||
expect(screen.getByTestId("environment-switch-overlay")).toHaveAttribute(
|
||||
"data-target",
|
||||
"Acme Local",
|
||||
"Local",
|
||||
);
|
||||
});
|
||||
|
||||
@@ -560,12 +553,7 @@ describe("BackendSelector", () => {
|
||||
renderWithProviders(
|
||||
<TestSeed
|
||||
onMount={(ctx) => {
|
||||
ctx.addBackend({
|
||||
name: "Local 1",
|
||||
host: "http://localhost:9000",
|
||||
apiKey: "k",
|
||||
kind: "local",
|
||||
});
|
||||
ctx.addBackend(SEED_LOCAL_1);
|
||||
}}
|
||||
>
|
||||
<BackendSelector />
|
||||
@@ -661,12 +649,7 @@ describe("BackendSelector", () => {
|
||||
return (
|
||||
<TestSeed
|
||||
onMount={(ctx) => {
|
||||
ctx.addBackend({
|
||||
name: "Local 1",
|
||||
host: "http://localhost:9000",
|
||||
apiKey: "k",
|
||||
kind: "local",
|
||||
});
|
||||
ctx.addBackend(SEED_LOCAL_1);
|
||||
}}
|
||||
>
|
||||
<BackendSelector />
|
||||
@@ -691,8 +674,9 @@ describe("BackendSelector", () => {
|
||||
</QueryClientProvider>,
|
||||
);
|
||||
|
||||
// Auto-switch lands on "Local 1"; click the seeded default to switch.
|
||||
const user = await openDropdown();
|
||||
await user.click(screen.getByText("Local 1"));
|
||||
await user.click(screen.getByText("Local"));
|
||||
|
||||
expect(await screen.findByTestId("automations-list")).toBeInTheDocument();
|
||||
});
|
||||
@@ -702,12 +686,7 @@ describe("BackendSelector", () => {
|
||||
return (
|
||||
<TestSeed
|
||||
onMount={(ctx) => {
|
||||
ctx.addBackend({
|
||||
name: "Local 1",
|
||||
host: "http://localhost:9000",
|
||||
apiKey: "k",
|
||||
kind: "local",
|
||||
});
|
||||
ctx.addBackend(SEED_LOCAL_1);
|
||||
}}
|
||||
>
|
||||
<BackendSelector />
|
||||
@@ -735,12 +714,13 @@ describe("BackendSelector", () => {
|
||||
const settingsButton = screen.getByTestId("backend-selector-settings-link");
|
||||
expect(settingsButton).toHaveAttribute("data-active", "true");
|
||||
|
||||
// Auto-switch lands on "Local 1"; click the seeded default to switch.
|
||||
const user = await openDropdown();
|
||||
await user.click(screen.getByText("Local 1"));
|
||||
await user.click(screen.getByText("Local"));
|
||||
|
||||
const wrapper = screen.getByTestId("backend-selector");
|
||||
const input = wrapper.querySelector("input") as HTMLInputElement;
|
||||
expect(input.value).toBe("Local 1");
|
||||
expect(input.value).toBe("Local");
|
||||
expect(screen.queryByTestId("home")).not.toBeInTheDocument();
|
||||
});
|
||||
|
||||
@@ -755,12 +735,7 @@ describe("BackendSelector", () => {
|
||||
renderWithProviders(
|
||||
<TestSeed
|
||||
onMount={(ctx) => {
|
||||
ctx.addBackend({
|
||||
name: "Local 1",
|
||||
host: "http://localhost:9000",
|
||||
apiKey: "k",
|
||||
kind: "local",
|
||||
});
|
||||
ctx.addBackend(SEED_LOCAL_1);
|
||||
}}
|
||||
>
|
||||
<BackendSelector />
|
||||
|
||||
@@ -76,6 +76,30 @@ describe("ActiveBackendProvider", () => {
|
||||
});
|
||||
});
|
||||
|
||||
// @spec BM-001 — Auto-switch to newly connected backend
|
||||
it("addBackend automatically switches the active backend to the newly added one", () => {
|
||||
const { result } = renderHook(() => useActiveBackendContext(), {
|
||||
wrapper: makeWrapper(),
|
||||
});
|
||||
|
||||
expect(result.current.active.backend.id).toBe(DEFAULT_LOCAL_BACKEND_ID);
|
||||
|
||||
let added: { id: string } | null = null;
|
||||
act(() => {
|
||||
added = result.current.addBackend({
|
||||
name: "OpenHands Cloud",
|
||||
host: "https://app.all-hands.dev",
|
||||
apiKey: "bearer-token",
|
||||
kind: "cloud",
|
||||
});
|
||||
});
|
||||
|
||||
expect(result.current.active.backend.id).toBe(added!.id);
|
||||
// Previous backends remain in the registry.
|
||||
expect(result.current.backends).toHaveLength(2);
|
||||
expect(result.current.backends.find((b) => b.id === DEFAULT_LOCAL_BACKEND_ID)).toBeDefined();
|
||||
});
|
||||
|
||||
it("setActive switches the active backend without touching unrelated React Query cache entries", () => {
|
||||
const queryClient = new QueryClient();
|
||||
queryClient.setQueryData(["dummy"], { value: 1 });
|
||||
|
||||
@@ -0,0 +1,4 @@
|
||||
# Backend Management Specs
|
||||
|
||||
## BM-001: Auto-switch on connect
|
||||
- [x] Adding a backend shall automatically switch the active selection to it.
|
||||
@@ -73,11 +73,13 @@ export function ActiveBackendProvider({
|
||||
[],
|
||||
);
|
||||
|
||||
// @spec BM-001 — Auto-switch to newly connected backend
|
||||
const addBackend = React.useCallback(
|
||||
(backend: Omit<Backend, "id">): Backend => {
|
||||
const next: Backend = { ...backend, id: generateId() };
|
||||
const list = [...getRegisteredBackends(), next];
|
||||
setRegisteredBackends(list);
|
||||
setActiveSelection({ backendId: next.id });
|
||||
return next;
|
||||
},
|
||||
[],
|
||||
|
||||
Reference in New Issue
Block a user