Flatten settings sidebar with a host picker (#1241)

* Flatten settings sidebar with a host picker Settings had two implicit tiers: app-level sections as flat sidebar rows, and per-host settings hidden behind a drill-in to one monolithic host page. This flattens both into a single sidebar with an "App" group and a "Host" group, where the host's settings are top-level rows scoped by a host picker. - Split the monolithic HostPage into four section pages: Connections, Orchestration (was the unnamed inject-tools + system-prompt block), Providers, and Daemon (status, updates, restart, remove). - Host picker reuses the canonical <Combobox> sidebar host switcher; defaults to the local host so a single-local-host user never thinks about "the daemon". - Host sections become real sub-routes (/settings/hosts/[id]/[section]); the old /settings/hosts/[id] redirects to the connections section, so existing deep links (pairing, workspace, open-project) keep working. - "Open providers" from the project screen now lands on the Providers section. * Fix e2e host-settings navigation for the flat sidebar The flat-settings refactor moved host settings onto separate section pages, but three e2e helpers still expected the old monolithic host page: - openDesktopSettings: the daemon-lifecycle card now lives on the Daemon section — navigate there before asserting it. - acp catalog: "Add provider" now lives on the Providers section — open that section before opening the add-provider modal. - openAddHostFlow: "Add host" is now an item inside the host picker (Combobox), not a standalone button — open the picker first. Test-only; no product changes. Fixes the 8 playwright failures on this branch.
2026-05-31 12:55:08 +08:00
parent c8ca503cc3
commit 0e3eb9cf75
13 changed files with 699 additions and 203 deletions
@@ -6,6 +6,7 @@ import {
  installAcpCatalogProvider,
  openAddProviderModal,
  openSettingsHost,
+  openSettingsHostSection,
 } from "./helpers/settings";

 const ACP_PROVIDER = {
@@ -18,6 +19,8 @@ test.describe("ACP provider catalog", () => {
    await gotoAppShell(page);
    await openSettings(page);
    await openSettingsHost(page, getServerId());
+    // Providers moved to their own host section; add-provider lives there now.
+    await openSettingsHostSection(page, getServerId(), "providers");
    await openAddProviderModal(page);

    await installAcpCatalogProvider(page, ACP_PROVIDER.name);
@@ -2,7 +2,7 @@ import { readFileSync } from "node:fs";
 import { expect, type Page } from "@playwright/test";
 import { openSettings } from "./app";
 import { getE2EDaemonPort } from "./daemon-port";
-import { openSettingsHost } from "./settings";
+import { openSettingsHost, openSettingsHostSection } from "./settings";

 interface DaemonApiStatus {
  version: string;
@@ -208,6 +208,9 @@ export async function injectDesktopBridge(page: Page, config: DesktopBridgeConfi
 export async function openDesktopSettings(page: Page, serverId: string): Promise<void> {
  await openSettings(page);
  await openSettingsHost(page, serverId);
+  // The daemon-lifecycle card moved to the Daemon section in the flat-settings
+  // layout; navigate there before asserting it.
+  await openSettingsHostSection(page, serverId, "daemon");
  await expect(page.getByTestId("host-page-daemon-lifecycle-card")).toBeVisible({
    timeout: 15_000,
  });
@@ -1,4 +1,5 @@
 import { expect, type Page } from "@playwright/test";
+import { TEST_HOST_LABEL } from "./daemon-registry";
 import { escapeRegex } from "./regex";
 import { getServerId } from "./server-id";

@@ -13,6 +14,8 @@ const SECTION_LABELS = {

 export type SettingsSection = keyof typeof SECTION_LABELS | "projects";

+type HostSection = "connections" | "orchestration" | "providers" | "daemon";
+
 export async function openSettingsSection(page: Page, section: SettingsSection): Promise<void> {
  const sidebar = page.getByTestId("settings-sidebar");
  await expect(sidebar).toBeVisible();
@@ -28,8 +31,22 @@ export async function openSettingsSection(page: Page, section: SettingsSection):
 }

 export async function openSettingsHost(page: Page, serverId: string): Promise<void> {
-  await page.getByTestId(`settings-host-entry-${serverId}`).click();
-  await expect(page.getByTestId(`settings-host-page-${serverId}`)).toBeVisible();
+  // Host sections are now flat top-level rows under the Host group. Navigate by
+  // clicking the Connections section row; the picker only matters when >1 host.
+  await page.getByTestId("settings-host-section-connections").click();
+  await expectHostSettingsUrl(page, serverId);
+  await expect(page.getByTestId("host-page-connections-card")).toBeVisible();
+}
+
+export async function openSettingsHostSection(
+  page: Page,
+  serverId: string,
+  section: HostSection,
+): Promise<void> {
+  await page.getByTestId(`settings-host-section-${section}`).click();
+  await expect(page).toHaveURL(
+    new RegExp(`/settings/hosts/${escapeRegex(encodeURIComponent(serverId))}/${section}$`),
+  );
 }

 export async function expectSettingsHeader(page: Page, title: string): Promise<void> {
@@ -37,6 +54,9 @@ export async function expectSettingsHeader(page: Page, title: string): Promise<v
 }

 export async function openAddHostFlow(page: Page): Promise<void> {
+  // "Add host" is now an item inside the host picker (a Combobox); open the
+  // picker first, then pick it. The picker renders whenever a host exists.
+  await page.getByTestId("settings-host-picker").click();
  await page.getByTestId("settings-add-host").click();
  await expect(page.getByText("Add connection", { exact: true })).toBeVisible();
 }
@@ -68,7 +88,7 @@ export async function expectCompactSettingsList(page: Page): Promise<void> {
  await expect(page.getByTestId("settings-sidebar")).toBeVisible();
  await expect(page.getByText("Theme", { exact: true })).toHaveCount(0);
  await expect(page.getByRole("button", { name: "Play test" })).toHaveCount(0);
-  await expect(page.locator('[data-testid^="settings-host-page-"]')).toHaveCount(0);
+  await expect(page.getByTestId("host-page-connections-card")).toHaveCount(0);
 }

 export async function expectSettingsSidebarVisible(page: Page): Promise<void> {
@@ -105,7 +125,7 @@ export async function clickSettingsBackToWorkspace(page: Page): Promise<void> {

 export async function expectHostSettingsUrl(page: Page, serverId: string): Promise<void> {
  await expect(page).toHaveURL(
-    new RegExp(`/settings/hosts/${escapeRegex(encodeURIComponent(serverId))}$`),
+    new RegExp(`/settings/hosts/${escapeRegex(encodeURIComponent(serverId))}/connections$`),
  );
 }

@@ -181,7 +201,11 @@ export async function expectHostLabelEditMode(page: Page, expectedLabel: string)
 export async function expectHostConnectionsCard(page: Page, port: string): Promise<void> {
  const card = page.getByTestId("host-page-connections-card");
  await expect(card).toBeVisible();
-  await expect(page.getByText("Connections", { exact: true })).toBeVisible();
+  // "Connections" appears three times on this page: the sidebar section row, the
+  // detail header title, and the SettingsSection heading above the card. Match
+  // the first to keep the heading assertion without tripping Playwright strict
+  // mode.
+  await expect(page.getByText("Connections", { exact: true }).first()).toBeVisible();
  await expect(
    card.getByText(new RegExp(`TCP \\((localhost|127\\.0\\.0\\.1):${port}\\)`)),
  ).toBeVisible();
@@ -193,14 +217,29 @@ export async function expectHostInjectMcpCard(page: Page): Promise<void> {
  await expect(card.getByRole("switch", { name: "Inject Paseo tools" })).toBeVisible();
 }

-export async function expectHostActionCards(page: Page): Promise<void> {
+export async function openHostSection(
+  page: Page,
+  serverId: string,
+  section: HostSection,
+): Promise<void> {
+  await openSettingsHostSection(page, serverId, section);
+}
+
+export async function expectHostActionCards(page: Page, serverId: string): Promise<void> {
+  // Restart + remove cards live on the Daemon section; providers moved to its
+  // own Providers section (asserted via expectHostProvidersCard).
+  await openSettingsHostSection(page, serverId, "daemon");
  await expect(page.getByTestId("host-page-restart-card")).toBeVisible();
  await expect(page.getByTestId("host-page-restart-button")).toBeVisible();
-  await expect(page.getByTestId("host-page-providers-card")).toBeVisible();
  await expect(page.getByTestId("host-page-remove-host-card")).toBeVisible();
  await expect(page.getByTestId("host-page-remove-host-button")).toBeVisible();
 }

+export async function expectHostProvidersCard(page: Page, serverId: string): Promise<void> {
+  await openSettingsHostSection(page, serverId, "providers");
+  await expect(page.getByTestId("host-page-providers-card")).toBeVisible();
+}
+
 export async function serveJson(page: Page, url: string, body: unknown): Promise<void> {
  await page.route(url, async (route) => {
    await route.fulfill({
@@ -244,27 +283,35 @@ export async function expectHostNoLocalOnlyRows(page: Page): Promise<void> {
 export async function expectRetiredSidebarSectionsAbsent(page: Page): Promise<void> {
  const sidebar = page.getByTestId("settings-sidebar");
  await expect(sidebar).toBeVisible();
-  await expect(sidebar.getByRole("button", { name: "Hosts", exact: true })).toHaveCount(0);
-  await expect(sidebar.getByRole("button", { name: "Providers", exact: true })).toHaveCount(0);
-  await expect(sidebar.getByRole("button", { name: "Pair device", exact: true })).toHaveCount(0);
-  await expect(sidebar.getByRole("button", { name: "Daemon", exact: true })).toHaveCount(0);
+
+  // App group rows remain top-level.
  await expect(sidebar.getByRole("button", { name: "General", exact: true })).toBeVisible();
  await expect(sidebar.getByRole("button", { name: "Diagnostics", exact: true })).toBeVisible();
  await expect(sidebar.getByRole("button", { name: "About", exact: true })).toBeVisible();
+
+  // Host group rows are now flat top-level sections (no drill-in).
+  await expect(sidebar.getByTestId("settings-host-section-connections")).toBeVisible();
+  await expect(sidebar.getByTestId("settings-host-section-orchestration")).toBeVisible();
+  await expect(sidebar.getByTestId("settings-host-section-providers")).toBeVisible();
+  await expect(sidebar.getByTestId("settings-host-section-daemon")).toBeVisible();
+
+  // The old per-host entry rows are replaced by the host picker.
+  await expect(sidebar.locator('[data-testid^="settings-host-entry-"]')).toHaveCount(0);
 }

-export async function expectHostPageVisible(page: Page, serverId: string): Promise<void> {
-  await expect(page.getByTestId(`settings-host-page-${serverId}`)).toBeVisible();
+export async function expectHostPageVisible(page: Page, _serverId: string): Promise<void> {
+  await expect(page.getByTestId("host-page-connections-card")).toBeVisible();
 }

-export async function expectLocalHostEntryFirst(page: Page, serverId: string): Promise<void> {
+export async function expectLocalHostEntryFirst(page: Page, _serverId: string): Promise<void> {
  const sidebar = page.getByTestId("settings-sidebar");
  await expect(sidebar).toBeVisible({ timeout: 15_000 });
-  await expect(sidebar.locator('[data-testid^="settings-host-entry-"]').first()).toHaveAttribute(
-    "data-testid",
-    `settings-host-entry-${serverId}`,
-  );
-  const localHostEntry = page.getByTestId(`settings-host-entry-${serverId}`);
-  await expect(localHostEntry.getByTestId("settings-host-local-marker")).toBeVisible();
-  await expect(localHostEntry.getByText("Local", { exact: true })).toBeVisible();
+
+  // Single-host fixture: the picker is a non-interactive chip (no dropdown to
+  // open) that surfaces the local host by its label. The "Local" marker only
+  // appears on dropdown rows in the multi-host case, which this fixture does not
+  // exercise.
+  const picker = sidebar.getByTestId("settings-host-picker");
+  await expect(picker).toBeVisible();
+  await expect(picker.getByText(TEST_HOST_LABEL, { exact: true })).toBeVisible();
 }
@@ -6,12 +6,14 @@ import { getServerId } from "./helpers/server-id";
 import {
  expectSettingsHeader,
  openSettingsHost,
+  openHostSection,
  expectHostLabelDisplayed,
  clickEditHostLabel,
  expectHostLabelEditMode,
  expectHostConnectionsCard,
  expectHostInjectMcpCard,
  expectHostActionCards,
+  expectHostProvidersCard,
  expectHostNoLocalOnlyRows,
  expectRetiredSidebarSectionsAbsent,
  expectHostPageVisible,
@@ -19,9 +21,7 @@ import {
 } from "./helpers/settings";

 test.describe("Settings host page", () => {
-  test("host page shows seeded label, connection endpoint, inject MCP toggle, and all action rows", async ({
-    page,
-  }) => {
+  test("connections section shows the seeded connection endpoint", async ({ page }) => {
    const serverId = getServerId();
    const port = getE2EDaemonPort();

@@ -29,11 +29,44 @@ test.describe("Settings host page", () => {
    await openSettings(page);
    await openSettingsHost(page, serverId);

-    await expectSettingsHeader(page, TEST_HOST_LABEL);
-    await expectHostLabelDisplayed(page);
+    await expectSettingsHeader(page, "Connections");
    await expectHostConnectionsCard(page, port);
+  });
+
+  test("orchestration section shows the inject MCP toggle", async ({ page }) => {
+    const serverId = getServerId();
+
+    await gotoAppShell(page);
+    await openSettings(page);
+    await openSettingsHost(page, serverId);
+
+    await openHostSection(page, serverId, "orchestration");
+    await expectSettingsHeader(page, "Orchestration");
    await expectHostInjectMcpCard(page);
-    await expectHostActionCards(page);
+  });
+
+  test("providers section shows the providers card", async ({ page }) => {
+    const serverId = getServerId();
+
+    await gotoAppShell(page);
+    await openSettings(page);
+    await openSettingsHost(page, serverId);
+
+    await expectHostProvidersCard(page, serverId);
+    await expectSettingsHeader(page, "Providers");
+  });
+
+  test("daemon section shows the host label and restart/remove action cards", async ({ page }) => {
+    const serverId = getServerId();
+
+    await gotoAppShell(page);
+    await openSettings(page);
+    await openSettingsHost(page, serverId);
+
+    await openHostSection(page, serverId, "daemon");
+    await expectSettingsHeader(page, "Daemon");
+    await expectHostLabelDisplayed(page);
+    await expectHostActionCards(page, serverId);
  });

  test("clicking the label pencil reveals the inline editor", async ({ page }) => {
@@ -42,13 +75,14 @@ test.describe("Settings host page", () => {
    await gotoAppShell(page);
    await openSettings(page);
    await openSettingsHost(page, serverId);
+    await openHostSection(page, serverId, "daemon");

    await expectHostLabelDisplayed(page);
    await clickEditHostLabel(page);
    await expectHostLabelEditMode(page, TEST_HOST_LABEL);
  });

-  test("host page does not render pair-device or daemon-lifecycle rows for a remote daemon", async ({
+  test("daemon section does not render pair-device or daemon-lifecycle rows for a remote daemon", async ({
    page,
  }) => {
    const serverId = getServerId();
@@ -56,19 +90,20 @@ test.describe("Settings host page", () => {
    await gotoAppShell(page);
    await openSettings(page);
    await openSettingsHost(page, serverId);
+    await openHostSection(page, serverId, "daemon");

    // TODO: add local-daemon fixture for positive Pair/Daemon coverage.
    await expectHostNoLocalOnlyRows(page);
  });

-  test("settings sidebar does not expose retired top-level sections", async ({ page }) => {
+  test("settings sidebar exposes the flat App and Host section rows", async ({ page }) => {
    await gotoAppShell(page);
    await openSettings(page);

    await expectRetiredSidebarSectionsAbsent(page);
  });

-  test("navigating to /settings/hosts/[serverId] directly renders the host page", async ({
+  test("navigating to /settings/hosts/[serverId] redirects to the connections section", async ({
    page,
  }) => {
    const serverId = getServerId();
@@ -77,9 +112,10 @@ test.describe("Settings host page", () => {
    await page.goto(`/settings/hosts/${encodeURIComponent(serverId)}`);

    await expectHostPageVisible(page, serverId);
-    await expectSettingsHeader(page, TEST_HOST_LABEL);
+    await expectSettingsHeader(page, "Connections");
+    await openHostSection(page, serverId, "daemon");
    await expectHostLabelDisplayed(page);
-    await expectHostActionCards(page);
+    await expectHostActionCards(page, serverId);
  });

  test("sidebar pins the local daemon host first with a Local marker", async ({ page }) => {
@@ -45,7 +45,7 @@ test.describe("Settings sidebar navigation", () => {
    await expectGeneralContent(page);
  });

-  test("/h/[serverId]/settings redirects to /settings/hosts/[serverId]", async ({ page }) => {
+  test("/h/[serverId]/settings redirects to the host connections section", async ({ page }) => {
    await gotoAppShell(page);
    await verifyLegacyHostSettingsRedirect(page);
  });
@@ -132,7 +132,9 @@ test.describe("Settings — compact master-detail", () => {
    await expectSettingsBackButton(page);
  });

-  test("tapping a host entry pushes /settings/hosts/[serverId]", async ({ page }) => {
+  test("tapping a host section row pushes /settings/hosts/[serverId]/connections", async ({
+    page,
+  }) => {
    await gotoAppShell(page);
    await openCompactSettings(page);