From 43ef9d77c10721319b34974cdc432c7d60f66a3a Mon Sep 17 00:00:00 2001 From: Mohamed Boudra Date: Tue, 5 May 2026 10:07:40 +0800 Subject: [PATCH] test(app/e2e): cover project-settings error-UX paths (Cluster G3) (#731) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * test(app/e2e): cover 5 error-UX paths + host indicator + script removal for project settings Add helpers/project-settings.ts DSL helpers and extend projects-settings.spec.ts with 5 new tests covering the error paths dropped by PR #725: - stale_project_config callout + disabled save + reload recovery - invalid_project_config read callout + reload after fix - write_failed callout + retry + reload recovery - single-host static indicator vs picker chip - script removal via kebab menu + confirm dialog * refactor(app/e2e): unslop project-settings helpers and spec - Fix removeProjectScript: derive trigger testID from row testID instead of using scoped locator (which was timing out) - Extract inline writeFile call in invalid-config test to restorePaseoConfig helper - Replace raw testID click in write_failed test with clickReloadProjectSettings - Remove writeFile from spec imports; drop defensive ?? "" fallback * test(app/e2e): add read-transport and offline no-target tests for project settings - Add read-transport failure test: WS-level drop during readProjectConfig triggers read-transport-callout; Reload retries until WS reconnects and refetch succeeds. - Add no-target test: WS drop after form load triggers NoEditableTarget via live connectionStatus check (useHostRuntimeSnapshot) on the selected host. - Hoist openProjects/editWorktreeSetup from spec body into project-settings helper. - Fix expectNoProjectSettingsError to accept optional timeout (needed for toPass loop). - Add isHostGone to renderContent: after readQuery errors are checked, offline/error connectionStatus renders NoEditableTarget without unmounting ProjectSettingsBody. * fixup(app/e2e): correct misleading comments on WS close → error-state mapping --- packages/app/e2e/helpers/project-settings.ts | 269 ++++++++++++++++++ packages/app/e2e/projects-settings.spec.ts | 195 +++++++++++-- .../src/screens/project-settings-screen.tsx | 16 ++ 3 files changed, 451 insertions(+), 29 deletions(-) create mode 100644 packages/app/e2e/helpers/project-settings.ts diff --git a/packages/app/e2e/helpers/project-settings.ts b/packages/app/e2e/helpers/project-settings.ts new file mode 100644 index 000000000..bed766cd5 --- /dev/null +++ b/packages/app/e2e/helpers/project-settings.ts @@ -0,0 +1,269 @@ +import { chmod, readFile, writeFile } from "node:fs/promises"; +import path from "node:path"; +import { expect, type Page } from "@playwright/test"; +import type { WebSocketRoute } from "@playwright/test"; +import { gotoAppShell, openSettings } from "./app"; + +// --- Navigation --- + +export async function openProjects(page: Page): Promise { + await gotoAppShell(page); + await openSettings(page); + await page.getByTestId("settings-projects").click(); + await expect(page).toHaveURL(/\/settings\/projects$/); +} + +export async function openProjectSettings(page: Page, projectName: string): Promise { + await page.getByRole("button", { name: `Edit ${projectName}`, exact: true }).click(); + await expect(page.getByRole("textbox", { name: "Worktree setup commands" })).toBeVisible({ + timeout: 30_000, + }); +} + +export async function navigateToProjectSettings(page: Page, projectName: string): Promise { + await page.getByRole("button", { name: `Edit ${projectName}`, exact: true }).click(); +} + +// --- Form interactions --- + +export async function editWorktreeSetup(page: Page, setupCommands: string[]): Promise { + await page + .getByRole("textbox", { name: "Worktree setup commands" }) + .fill(setupCommands.join("\n")); +} + +export async function clickSaveProjectSettings(page: Page): Promise { + await page.getByRole("button", { name: "Save project config" }).click(); +} + +export async function clickRetryProjectSettingsSave(page: Page): Promise { + // action-0 is always "Try again"; action-1 is always "Reload". + // The write-failed callout renders these two buttons in a fixed order. + await page.getByTestId("write-failed-callout-action-0").click(); +} + +export async function clickReloadProjectSettings(page: Page): Promise { + // Scope to the active error callout so the locator is unambiguous. + // At most one error callout renders at a time. + await page.locator('[data-testid$="-callout"]').getByRole("button", { name: "Reload" }).click(); +} + +// --- Error-state assertions --- + +type ErrorKind = "stale" | "invalid" | "write_failed" | "transport" | "read_failed"; + +const errorCalloutTestId: Record = { + stale: "stale-callout", + invalid: "invalid-callout", + write_failed: "write-failed-callout", + transport: "read-transport-callout", + read_failed: "read-failed-callout", +}; + +export async function expectProjectSettingsError(page: Page, kind: ErrorKind): Promise { + await expect(page.getByTestId(errorCalloutTestId[kind])).toBeVisible({ timeout: 15_000 }); +} + +export async function expectNoProjectSettingsError( + page: Page, + kind: ErrorKind, + timeout = 15_000, +): Promise { + await expect(page.getByTestId(errorCalloutTestId[kind])).not.toBeVisible({ timeout }); +} + +export async function expectWriteFailedCalloutActions(page: Page): Promise { + await expect(page.getByTestId("write-failed-callout-action-0")).toHaveText("Try again"); + await expect(page.getByTestId("write-failed-callout-action-1")).toHaveText("Reload"); +} + +export async function expectSaveButtonDisabled(page: Page): Promise { + await expect(page.getByRole("button", { name: "Save project config" })).toBeDisabled(); +} + +// --- Form-state assertions --- + +export async function expectProjectSettingsFormVisible(page: Page): Promise { + await expect(page.getByRole("textbox", { name: "Worktree setup commands" })).toBeVisible({ + timeout: 15_000, + }); +} + +export async function expectProjectSettingsFormHidden(page: Page): Promise { + await expect(page.getByRole("textbox", { name: "Worktree setup commands" })).not.toBeVisible(); +} + +export async function expectNoEditableTarget(page: Page): Promise { + await expect(page.getByTestId("project-settings-back-button")).toBeVisible({ timeout: 30_000 }); +} + +// --- Host-section assertions --- + +export async function expectHostIndicatorVisible(page: Page): Promise { + await expect(page.getByTestId("host-indicator")).toBeVisible(); +} + +export async function expectHostPickerHidden(page: Page): Promise { + await expect(page.getByTestId("host-picker")).not.toBeVisible(); +} + +// --- Script-list assertions and interactions --- + +// Counts only row Views, not kebab-trigger elements (which share the "script-row-" +// prefix but contain "-menu-"). +export async function expectScriptRowCount(page: Page, count: number): Promise { + await expect( + page + .getByTestId("scripts-list") + .locator('[data-testid^="script-row-"]:not([data-testid*="-menu-"])'), + ).toHaveCount(count); +} + +export async function expectEmptyScriptList(page: Page): Promise { + await expect(page.getByText("No scripts yet.")).toBeVisible(); +} + +export async function removeProjectScript(page: Page, scriptName: string): Promise { + const row = page + .getByTestId("scripts-list") + .locator('[data-testid^="script-row-"]:not([data-testid*="-menu-"])') + .filter({ hasText: scriptName }) + .first(); + // DropdownMenuTrigger renders as a Pressable (no role="button"); derive its testID + // from the row's testID to avoid scoped locator unreliability. + const id = (await row.getAttribute("data-testid"))!.replace("script-row-", ""); + await page.getByTestId(`script-row-menu-${id}`).click(); + page.once("dialog", (dialog) => void dialog.accept()); + await page.getByRole("button", { name: "Remove" }).click(); +} + +// --- File manipulation --- + +export async function corruptPaseoConfig(repoPath: string): Promise { + await writeFile(path.join(repoPath, "paseo.json"), "{not valid json}"); +} + +export async function bumpPaseoConfigOnDisk(repoPath: string): Promise { + const configPath = path.join(repoPath, "paseo.json"); + const raw = await readFile(configPath, "utf8"); + const config = JSON.parse(raw) as Record; + config._bump = Date.now(); + await writeFile(configPath, JSON.stringify(config, null, 2) + "\n"); +} + +export async function restorePaseoConfig( + repoPath: string, + config: Record, +): Promise { + await writeFile(path.join(repoPath, "paseo.json"), JSON.stringify(config, null, 2) + "\n"); +} + +// The daemon writes atomically via a temp file + rename, so blocking writes requires +// removing write permission from the *directory*, not just the file. +export async function blockPaseoConfigWrites(repoPath: string): Promise { + await chmod(repoPath, 0o555); +} + +export async function unblockPaseoConfigWrites(repoPath: string): Promise { + await chmod(repoPath, 0o755); +} + +// --- WebSocket helpers --- + +function buildDaemonPortPattern(): RegExp { + const port = process.env.E2E_DAEMON_PORT; + if (!port) throw new Error("E2E_DAEMON_PORT not set — globalSetup must run first"); + return new RegExp(`:${port.replace(/[.*+?^${}()|[\]\\]/g, "\\$&")}\\b`); +} + +// Proxies all daemon WS traffic transparently until a read_project_config_request +// is seen, then closes that connection (triggering readQuery.isError). Subsequent +// connections pass through so the Reload action can succeed. +export async function installReadTransportFailure(page: Page): Promise { + let armed = true; + + await page.routeWebSocket(buildDaemonPortPattern(), (ws) => { + const server = ws.connectToServer(); + + ws.onMessage((message) => { + if (armed && typeof message === "string") { + try { + const envelope = JSON.parse(message) as { + type?: string; + message?: { type?: string }; + }; + if ( + envelope.type === "session" && + envelope.message?.type === "read_project_config_request" + ) { + armed = false; + void ws.close({ code: 1001 }); + return; + } + } catch { + // binary or malformed frame — pass through + } + } + try { + server.send(message); + } catch { + // server socket already closed + } + }); + + server.onMessage((message) => { + try { + ws.send(message); + } catch { + // client socket already closed + } + }); + }); +} + +// Installs a transparent WS proxy that can later drop all active daemon connections +// and block new ones. Code 1001 (Going Away) without reason triggers "error" state +// in DaemonClient due to describeTransportClose returning a non-empty string. +export async function installDaemonConnectionGate( + page: Page, +): Promise<{ drop: () => Promise }> { + let acceptingConnections = true; + const activeSockets = new Set(); + + await page.routeWebSocket(buildDaemonPortPattern(), (ws) => { + if (!acceptingConnections) { + void ws.close({ code: 1001 }); + return; + } + + activeSockets.add(ws); + const server = ws.connectToServer(); + + ws.onMessage((message) => { + if (!acceptingConnections) return; + try { + server.send(message); + } catch { + activeSockets.delete(ws); + } + }); + + server.onMessage((message) => { + if (!acceptingConnections) return; + try { + ws.send(message); + } catch { + activeSockets.delete(ws); + } + }); + }); + + return { + async drop(): Promise { + acceptingConnections = false; + const sockets = Array.from(activeSockets); + activeSockets.clear(); + await Promise.all(sockets.map((ws) => ws.close({ code: 1001 }).catch(() => undefined))); + }, + }; +} diff --git a/packages/app/e2e/projects-settings.spec.ts b/packages/app/e2e/projects-settings.spec.ts index 77ebc5edd..c01f336a9 100644 --- a/packages/app/e2e/projects-settings.spec.ts +++ b/packages/app/e2e/projects-settings.spec.ts @@ -1,9 +1,36 @@ -import { readFile } from "node:fs/promises"; +import { chmod, readFile } from "node:fs/promises"; import path from "node:path"; -import { expect, test as base, type Page } from "./fixtures"; -import { gotoAppShell, openSettings } from "./helpers/app"; +import { expect, test as base } from "./fixtures"; import { connectNewWorkspaceDaemonClient, openProjectViaDaemon } from "./helpers/new-workspace"; import { createTempGitRepo } from "./helpers/workspace"; +import { + blockPaseoConfigWrites, + bumpPaseoConfigOnDisk, + clickReloadProjectSettings, + clickRetryProjectSettingsSave, + clickSaveProjectSettings, + corruptPaseoConfig, + editWorktreeSetup, + expectEmptyScriptList, + expectHostIndicatorVisible, + expectHostPickerHidden, + expectNoEditableTarget, + expectNoProjectSettingsError, + expectProjectSettingsError, + expectProjectSettingsFormHidden, + expectProjectSettingsFormVisible, + expectSaveButtonDisabled, + expectScriptRowCount, + expectWriteFailedCalloutActions, + installDaemonConnectionGate, + installReadTransportFailure, + navigateToProjectSettings, + openProjectSettings, + openProjects, + removeProjectScript, + restorePaseoConfig, + unblockPaseoConfigWrites, +} from "./helpers/project-settings"; const updatedSetup = ["npm install", "npm run build"]; @@ -48,6 +75,9 @@ const test = base.extend({ }); await client.close(); + // Defensive: restore directory write permission in case the test left it blocked + // (write_failed test), so that repo.cleanup() can remove files inside. + await chmod(repo.path, 0o755).catch(() => undefined); await repo.cleanup(); }, gitlabRemoteProject: async ({ page: _page }, provide) => { @@ -68,30 +98,6 @@ const test = base.extend({ }, }); -async function openProjects(page: Page): Promise { - await gotoAppShell(page); - await openSettings(page); - await page.getByTestId("settings-projects").click(); - await expect(page).toHaveURL(/\/settings\/projects$/); -} - -async function openProjectSettings(page: Page, projectName: string): Promise { - await page.getByRole("button", { name: `Edit ${projectName}`, exact: true }).click(); - await expect(page.getByRole("textbox", { name: "Worktree setup commands" })).toBeVisible({ - timeout: 30_000, - }); -} - -async function editWorktreeSetup(page: Page, setupCommands: string[]): Promise { - await page - .getByRole("textbox", { name: "Worktree setup commands" }) - .fill(setupCommands.join("\n")); -} - -async function saveProjectConfig(page: Page): Promise { - await page.getByRole("button", { name: "Save project config" }).click(); -} - async function expectProjectConfigSaved(project: ProjectsSettingsProject): Promise { await expect .poll( @@ -133,7 +139,7 @@ test.describe("Projects settings", () => { await openProjects(page); await openProjectSettings(page, editableProject.name); await editWorktreeSetup(page, updatedSetup); - await saveProjectConfig(page); + await clickSaveProjectSettings(page); await expectProjectConfigSaved(editableProject); }); @@ -145,7 +151,138 @@ test.describe("Projects settings", () => { await openProjects(page); await openProjectSettings(page, gitlabRemoteProject.name); await editWorktreeSetup(page, updatedSetup); - await saveProjectConfig(page); + await clickSaveProjectSettings(page); await expectProjectConfigSaved(gitlabRemoteProject); }); }); + +test.describe("Projects settings — error UX", () => { + test("stale-write callout appears on save, disables save, and reload clears it", async ({ + page, + editableProject, + }) => { + await openProjects(page); + await openProjectSettings(page, editableProject.name); + + // Bump the file on disk so the daemon detects a revision mismatch on save. + await bumpPaseoConfigOnDisk(editableProject.path); + + await clickSaveProjectSettings(page); + + await expectProjectSettingsError(page, "stale"); + await expectSaveButtonDisabled(page); + + await clickReloadProjectSettings(page); + + await expectNoProjectSettingsError(page, "stale"); + await expectProjectSettingsFormVisible(page); + }); + + test("invalid paseo.json shows read-error callout, reload after fix shows form", async ({ + page, + editableProject, + }) => { + await corruptPaseoConfig(editableProject.path); + + await openProjects(page); + await navigateToProjectSettings(page, editableProject.name); + + await expectProjectSettingsError(page, "invalid"); + await expectProjectSettingsFormHidden(page); + + // Restore a valid config so the reload succeeds. + await restorePaseoConfig(editableProject.path, initialPaseoConfig); + + await clickReloadProjectSettings(page); + + await expectNoProjectSettingsError(page, "invalid"); + await expectProjectSettingsFormVisible(page); + }); + + test("write_failed callout appears on save with blocked directory, retry re-attempts, reload clears it", async ({ + page, + editableProject, + }) => { + await openProjects(page); + await openProjectSettings(page, editableProject.name); + + await blockPaseoConfigWrites(editableProject.path); + + await clickSaveProjectSettings(page); + + await expectProjectSettingsError(page, "write_failed"); + await expectWriteFailedCalloutActions(page); + + await clickRetryProjectSettingsSave(page); + await expectProjectSettingsError(page, "write_failed"); + + await unblockPaseoConfigWrites(editableProject.path); + await clickReloadProjectSettings(page); + await expectNoProjectSettingsError(page, "write_failed"); + await expectProjectSettingsFormVisible(page); + }); + + test("read-transport failure shows callout, reload recovers", async ({ + page, + editableProject, + }) => { + // Drop the WS connection the moment a read_project_config_request is sent. + // Subsequent connections are proxied transparently so Reload can succeed. + await installReadTransportFailure(page); + + await openProjects(page); + await navigateToProjectSettings(page, editableProject.name); + + await expectProjectSettingsError(page, "transport"); + await expectProjectSettingsFormHidden(page); + + // The client reconnects after a ~1.5 s backoff; retry Reload until refetch succeeds. + await expect(async () => { + await clickReloadProjectSettings(page); + await expectNoProjectSettingsError(page, "transport", 3_000); + }).toPass({ timeout: 15_000 }); + await expectProjectSettingsFormVisible(page); + }); + + test("project settings shows no-target state when daemon connection drops", async ({ + page, + editableProject, + }) => { + const gate = await installDaemonConnectionGate(page); + + await openProjects(page); + await openProjectSettings(page, editableProject.name); + + // Closing with code 1001 (Going Away) transitions DaemonClient to "error" state. + // The NoEditableTarget UI renders via isHostGone check regardless of state. + await gate.drop(); + + await expectNoEditableTarget(page); + }); + + test("single-host project renders static host indicator, not a picker chip", async ({ + page, + editableProject, + }) => { + await openProjects(page); + await openProjectSettings(page, editableProject.name); + + await expectHostIndicatorVisible(page); + await expectHostPickerHidden(page); + }); + + test("script removal via kebab menu removes the row from the form", async ({ + page, + editableProject, + }) => { + await openProjects(page); + await openProjectSettings(page, editableProject.name); + + await expectScriptRowCount(page, 1); + + await removeProjectScript(page, "dev"); + + await expectScriptRowCount(page, 0); + await expectEmptyScriptList(page); + }); +}); diff --git a/packages/app/src/screens/project-settings-screen.tsx b/packages/app/src/screens/project-settings-screen.tsx index cf1fa27a5..1a2253cab 100644 --- a/packages/app/src/screens/project-settings-screen.tsx +++ b/packages/app/src/screens/project-settings-screen.tsx @@ -71,6 +71,12 @@ export default function ProjectSettingsScreen({ projectKey }: ProjectSettingsScr } }, [editableHosts, selectedServerId]); + const selectedSnapshot = useHostRuntimeSnapshot(selectedServerId); + const isHostGone = + Boolean(selectedServerId) && + (selectedSnapshot?.connectionStatus === "offline" || + selectedSnapshot?.connectionStatus === "error"); + const selectedHost = editableHosts.find((host) => host.serverId === selectedServerId); const client = useHostRuntimeClient(selectedHost?.serverId ?? ""); @@ -85,6 +91,7 @@ export default function ProjectSettingsScreen({ projectKey }: ProjectSettingsScr selectedHost={selectedHost} onSelectHost={setSelectedServerId} client={client} + isHostGone={isHostGone} /> ); } @@ -139,6 +146,7 @@ interface ProjectSettingsBodyProps { selectedHost: ProjectHostEntry; onSelectHost: (serverId: string) => void; client: DaemonClient; + isHostGone: boolean; } function ProjectSettingsBody({ @@ -147,6 +155,7 @@ function ProjectSettingsBody({ selectedHost, onSelectHost, client, + isHostGone, }: ProjectSettingsBodyProps) { const queryKey = useMemo( () => ["project-config", selectedHost.serverId, selectedHost.repoRoot] as const, @@ -192,6 +201,7 @@ function ProjectSettingsBody({ client, onReload: handleReload, hasMultipleHosts, + isHostGone, })} ); @@ -207,6 +217,7 @@ interface RenderContentInput { client: DaemonClient; onReload: () => void; hasMultipleHosts: boolean; + isHostGone: boolean; } function renderContent({ @@ -219,6 +230,7 @@ function renderContent({ client, onReload, hasMultipleHosts, + isHostGone, }: RenderContentInput) { if (readQuery.isLoading) { return ( @@ -250,6 +262,10 @@ function renderContent({ ); } + if (isHostGone) { + return ; + } + if (!loadedConfig) { return (