diff --git a/packages/app/e2e/browser/sidebar-project-name-whitespace.spec.ts b/packages/app/e2e/browser/sidebar-project-name-whitespace.spec.ts new file mode 100644 index 0000000000..3197cd060a --- /dev/null +++ b/packages/app/e2e/browser/sidebar-project-name-whitespace.spec.ts @@ -0,0 +1,77 @@ +import { randomUUID } from "node:crypto"; +import { mkdir, mkdtemp, rm, writeFile } from "node:fs/promises"; +import { tmpdir } from "node:os"; +import path from "node:path"; +import type { Page } from "@playwright/test"; +import { expect, test } from "../support/fixtures"; +import { gotoAppShell } from "../support/helpers/app"; +import { projectEquivalenceViewKey } from "../support/helpers/project-view-key"; +import { connectSeedClient } from "../support/helpers/seed-client"; + +interface SeededProject { + projectKey: string; + cleanup(): Promise; +} + +/** + * Adds a project whose directory name ends in a space, which `seedWorkspace` + * cannot produce because `mkdtemp` appends the random suffix. The project is + * owned until `cleanup`, which removes the daemon record before the directory. + */ +async function seedProjectNamedWithTrailingSpace(): Promise { + const parentDirectory = await mkdtemp(path.join(tmpdir(), "paseo-e2e-trailing-space-")); + const directoryPath = path.join(parentDirectory, `Reklamation-${randomUUID().slice(0, 8)} `); + await mkdir(directoryPath); + await writeFile(path.join(directoryPath, "README.md"), "# Trailing space\n"); + + const client = await connectSeedClient(); + const removeParentDirectory = () => rm(parentDirectory, { recursive: true, force: true }); + try { + const added = await client.addProject(directoryPath); + expect(added.error).toBeNull(); + expect(added.project?.projectRootPath).toBe(directoryPath); + const project = added.project; + if (!project?.projectKey) { + throw new Error(`The daemon added ${directoryPath} without a project key`); + } + return { + projectKey: project.projectKey, + cleanup: async () => { + await client.removeProject(project.projectId).catch(() => undefined); + await client.close().catch(() => undefined); + await removeParentDirectory().catch(() => undefined); + }, + }; + } catch (error) { + await client.close().catch(() => undefined); + await removeParentDirectory().catch(() => undefined); + throw error; + } +} + +async function expectProjectRow(page: Page, projectKey: string): Promise { + const row = page.getByTestId(`sidebar-project-row-${projectEquivalenceViewKey(projectKey)}`); + await expect(row).toBeVisible({ timeout: 60_000 }); +} + +async function expectAppShellRendered(page: Page): Promise { + await expect(page.getByText("Paseo ran into a problem.")).toHaveCount(0); +} + +// A project view key carries the project's path, so a directory whose name ends +// in a space produces a key that ends in a space. The sidebar order store used +// to trim it on write, which left the reconcile effect writing a new order on +// every render until React tore the app down with "Maximum update depth +// exceeded" on every load (#4880). +test("a project whose folder name ends in a space renders in the sidebar", async ({ page }) => { + const project = await seedProjectNamedWithTrailingSpace(); + + try { + await gotoAppShell(page); + + await expectProjectRow(page, project.projectKey); + await expectAppShellRendered(page); + } finally { + await project.cleanup(); + } +}); diff --git a/packages/app/src/stores/sidebar-order-store.test.ts b/packages/app/src/stores/sidebar-order-store.test.ts index c58931ca69..a68ca206f9 100644 --- a/packages/app/src/stores/sidebar-order-store.test.ts +++ b/packages/app/src/stores/sidebar-order-store.test.ts @@ -1,5 +1,9 @@ -import { describe, expect, it } from "vitest"; -import { migrateSidebarOrderState } from "./sidebar-order-store"; +import { beforeEach, describe, expect, it } from "vitest"; +import { + computeSidebarOrderUpdates, + type SidebarProjectEntry, +} from "@/hooks/sidebar-workspaces-view-model"; +import { migrateSidebarOrderState, useSidebarOrderStore } from "./sidebar-order-store"; describe("migrateSidebarOrderState", () => { it("prefixes legacy per-server workspace order with the source server id", () => { @@ -31,3 +35,71 @@ describe("migrateSidebarOrderState", () => { expect(migrated.pinnedWorkspaceOrder).toEqual(["host-a:one", "host-b:two"]); }); }); + +// A directory whose name ends in a space produces a view key that ends in a +// space. Trimming it on write used to make the sidebar's reconcile effect loop +// forever, crashing the app with React error #185 (see #4880). +describe("sidebar order keys that end in whitespace", () => { + const PROJECT_KEY = "host:srv-1:/home/u/Reklamation "; + + beforeEach(() => { + useSidebarOrderStore.setState({ + projectOrder: [], + pinnedWorkspaceOrder: [], + workspaceOrderByProject: {}, + }); + }); + + it("stores a project key exactly as given", () => { + useSidebarOrderStore.getState().setProjectOrder([PROJECT_KEY]); + + expect(useSidebarOrderStore.getState().projectOrder).toEqual([PROJECT_KEY]); + }); + + it("scopes a workspace order under the project key the sidebar indexes by", () => { + useSidebarOrderStore.getState().setWorkspaceOrder(PROJECT_KEY, ["srv-1:main"]); + + // use-sidebar-workspaces-list reads workspaceOrderByProject by view key + // directly, so the scope has to be stored under that same key. + expect(useSidebarOrderStore.getState().workspaceOrderByProject[PROJECT_KEY]).toEqual([ + "srv-1:main", + ]); + expect(useSidebarOrderStore.getState().getWorkspaceOrder(PROJECT_KEY)).toEqual(["srv-1:main"]); + }); + + it("settles the sidebar order reconcile instead of rewriting it forever", () => { + const projects: SidebarProjectEntry[] = [ + { + viewKey: PROJECT_KEY, + projectName: "Reklamation ", + projectKind: "non_git", + iconWorkingDir: "/home/u/Reklamation ", + hosts: [], + workspaces: [], + }, + ]; + const runPass = () => + computeSidebarOrderUpdates({ + projects, + persistedProjectOrder: useSidebarOrderStore.getState().projectOrder, + getWorkspaceOrder: (key) => useSidebarOrderStore.getState().getWorkspaceOrder(key), + }); + + // First pass adds the newly visible project. + const first = runPass(); + expect(first.projectOrder).toEqual([PROJECT_KEY]); + useSidebarOrderStore.getState().setProjectOrder(first.projectOrder ?? []); + + // Second pass must find nothing left to do; otherwise the effect that + // applies these updates re-runs on every render. + expect(runPass().projectOrder).toBeNull(); + }); + + it("still trims keys when migrating persisted state", () => { + const migrated = migrateSidebarOrderState({ + projectOrder: [" host-a:one ", "host-a:one"], + }); + + expect(migrated.projectOrder).toEqual(["host-a:one"]); + }); +}); diff --git a/packages/app/src/stores/sidebar-order-store.ts b/packages/app/src/stores/sidebar-order-store.ts index 27e7a190c8..10831480f8 100644 --- a/packages/app/src/stores/sidebar-order-store.ts +++ b/packages/app/src/stores/sidebar-order-store.ts @@ -38,6 +38,8 @@ interface SidebarWorkspaceOrderScope { projectViewKey: string; } +// Trims each key. Only for persisted state read at migration time, where a +// stray space is an artifact of an older format rather than part of the key. function normalizeKeys(keys: string[]): string[] { const seen = new Set(); const normalized: string[] = []; @@ -54,6 +56,28 @@ function normalizeKeys(keys: string[]): string[] { return normalized; } +/** + * Drops blank keys and duplicates but keeps each key exactly as given. View + * keys embed a project's path, so a directory whose name ends in a space + * produces a key that ends in a space. Trimming it stores a key that can never + * match the one the sidebar looks up, so the caller sees its key as missing, + * writes it again, and the effect that reconciles the order never settles. + */ +function dedupeKeys(keys: string[]): string[] { + const seen = new Set(); + const deduped: string[] = []; + + for (const key of keys) { + if (!key.trim() || seen.has(key)) { + continue; + } + seen.add(key); + deduped.push(key); + } + + return deduped; +} + function normalizeWorkspaceOrderByProject( workspaceOrderByProject: Record | undefined, ): Record { @@ -134,27 +158,22 @@ export const useSidebarOrderStore = create()( workspaceOrderByProject: {}, getProjectOrder: () => get().projectOrder, setProjectOrder: (keys) => { - const normalized = normalizeKeys(keys); - set({ projectOrder: normalized }); + set({ projectOrder: dedupeKeys(keys) }); }, getPinnedWorkspaceOrder: () => get().pinnedWorkspaceOrder, setPinnedWorkspaceOrder: (keys) => { - const normalized = normalizeKeys(keys); - set({ pinnedWorkspaceOrder: normalized }); + set({ pinnedWorkspaceOrder: dedupeKeys(keys) }); }, getWorkspaceOrder: (projectViewKey) => { - const scope = projectViewKey.trim(); - if (!scope) return []; - return get().workspaceOrderByProject[scope] ?? []; + if (!projectViewKey.trim()) return []; + return get().workspaceOrderByProject[projectViewKey] ?? []; }, setWorkspaceOrder: (projectViewKey, keys) => { - const scope = projectViewKey.trim(); - if (!scope) return; - const normalized = normalizeKeys(keys); + if (!projectViewKey.trim()) return; set((state) => ({ workspaceOrderByProject: { ...state.workspaceOrderByProject, - [scope]: normalized, + [projectViewKey]: dedupeKeys(keys), }, })); }, diff --git a/packages/app/src/stores/sidebar-view-store.test.ts b/packages/app/src/stores/sidebar-view-store.test.ts index b9df715fa2..902294dd2d 100644 --- a/packages/app/src/stores/sidebar-view-store.test.ts +++ b/packages/app/src/stores/sidebar-view-store.test.ts @@ -1,4 +1,4 @@ -import { beforeEach, describe, expect, it, vi } from "vitest"; +import { beforeEach, describe, expect, it } from "vitest"; import type { StateStorage } from "zustand/middleware"; import { createSidebarViewStorage, @@ -8,14 +8,6 @@ import { useSidebarViewStore, } from "./sidebar-view-store"; -vi.mock("@react-native-async-storage/async-storage", () => ({ - default: { - getItem: vi.fn().mockResolvedValue(null), - setItem: vi.fn().mockResolvedValue(undefined), - removeItem: vi.fn().mockResolvedValue(undefined), - }, -})); - interface MemoryStorage extends StateStorage> { reads: string[]; } diff --git a/packages/app/test-stubs/async-storage.ts b/packages/app/test-stubs/async-storage.ts new file mode 100644 index 0000000000..68cc5dd114 --- /dev/null +++ b/packages/app/test-stubs/async-storage.ts @@ -0,0 +1,45 @@ +/** + * AsyncStorage has no implementation under Node: its default export reaches for + * the native module and every call rejects, so any test that touches a + * persisted store emits unhandled rejections from the persist middleware. + * + * This is a real in-memory key-value store with the same contract, not a + * recording double, so a test can read back what a store wrote. + */ +const entries = new Map(); + +export function clearAsyncStorageStub(): void { + entries.clear(); +} + +const AsyncStorageStub = { + getItem: (key: string): Promise => Promise.resolve(entries.get(key) ?? null), + setItem: (key: string, value: string): Promise => { + entries.set(key, value); + return Promise.resolve(); + }, + removeItem: (key: string): Promise => { + entries.delete(key); + return Promise.resolve(); + }, + getAllKeys: (): Promise => Promise.resolve([...entries.keys()]), + // `provider-snapshot-cache` declares its own `ProviderSnapshotStorage` port and + // hands it the real AsyncStorage, so a stub that stops at the single-key calls + // fails any test that reaches the provider cache with `multiGet is not a function`. + multiGet: (keys: readonly string[]): Promise> => + Promise.resolve(keys.map((key) => [key, entries.get(key) ?? null] as [string, string | null])), + multiSet: (pairs: ReadonlyArray): Promise => { + for (const [key, value] of pairs) entries.set(key, value); + return Promise.resolve(); + }, + multiRemove: (keys: readonly string[]): Promise => { + for (const key of keys) entries.delete(key); + return Promise.resolve(); + }, + clear: (): Promise => { + entries.clear(); + return Promise.resolve(); + }, +}; + +export default AsyncStorageStub; diff --git a/packages/app/vitest.config.ts b/packages/app/vitest.config.ts index 5e560be880..521fb6dbe0 100644 --- a/packages/app/vitest.config.ts +++ b/packages/app/vitest.config.ts @@ -185,6 +185,12 @@ export default defineConfig({ find: /^expo-linking$/, replacement: path.resolve(__dirname, "test-stubs/expo-linking.ts"), }, + // No Node implementation: every call on the real module rejects, so a test + // touching a persisted store drowns in unhandled rejections. + { + find: /^@react-native-async-storage\/async-storage$/, + replacement: path.resolve(__dirname, "test-stubs/async-storage.ts"), + }, { find: /^lucide-react-native$/, replacement: path.resolve(__dirname, "test-stubs/lucide-react-native.ts"),