Keep a project in the sidebar when its folder name ends in a space (#5205)

* fix(app): keep sidebar order keys that end in whitespace

A project view key embeds 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 trimmed every key on write, so the stored key could never match
the key the sidebar looks up. computeSidebarOrderUpdates saw the project
as missing from the order on every pass, appended it, and the store
trimmed it away again, setting a fresh array each time. The effect that
applies those updates re-ran on every render until React gave up with
error #185, and the app failed to load until the folder was renamed.

Keep keys verbatim on the runtime setters, dropping only blank keys and
duplicates. Migration of persisted state still trims, where a stray
space is an artifact of an older format rather than part of the key.

Fixes #4880

* test(app): give AsyncStorage a real in-memory stub instead of mocking it

`docs/testing.md` bans `vi.mock`, and the test added here mocked
`@react-native-async-storage/async-storage` only because the module has no
Node implementation: its default export reaches for the native module, so
every write from the persist middleware rejects and a test that touches a
persisted store drowns in unhandled rejections.

That is exactly the case `test-stubs/` exists for, reached through the
resolve.alias in `vitest.config.ts` the same way react-native-unistyles,
react-native-svg and expo-linking are. The stub is a real in-memory
key-value store with the same contract, so a test can read back what a
store wrote rather than inspect a recording double.

`sidebar-view-store.test.ts` carried the same mock for the same reason and
no longer needs it either.

* test(app): give the AsyncStorage stub the multi-key calls the app uses

`provider-snapshot-cache` declares its own `ProviderSnapshotStorage` port and
is handed the real AsyncStorage, so it calls `multiGet` and `multiRemove`
during reconciliation and cleanup. The stub stopped at the single-key calls,
which is fine until a test reaches the provider cache and then fails with
`storage.multiGet is not a function`.

`multiSet` comes along with them so the three multi-key calls behave as a set
rather than leaving the next caller to notice the gap the same way.

* test(app): cover a project folder name ending in a space end to end

Adds a browser regression for #4880: the daemon accepts a directory whose
name ends in a space, and the sidebar has to render its project row instead
of tearing the app down with "Maximum update depth exceeded" on every load.

* test(app): move the trailing-space spec setup behind a seeded-project helper

docs/testing.md asks for E2E bodies that read like plain English with the
setup in helpers, and for helpers that own the project record until cleanup.
The spec did its directory creation, daemon seeding, and removal inline.

seedProjectNamedWithTrailingSpace owns the directory and the daemon project
and returns a cleanup, the same shape seedWorkspace returns, which cannot be
used here because mkdtemp appends the random suffix and so cannot produce a
name that ends in a space. The body is now goto, assert the row, assert no
error screen.

The row is still located by its sidebar-project-row test id, which is how
every other sidebar spec and the shared sidebar helpers address a project row.

---------

Co-authored-by: L4XB <lukas.buck@e-mail.de>
Co-authored-by: paseo-bot[bot] <paseo-bot[bot]@users.noreply.github.com>

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<void>;
+}
+
+/**
+ * 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<SeededProject> {
+  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<void> {
+  const row = page.getByTestId(`sidebar-project-row-${projectEquivalenceViewKey(projectKey)}`);
+  await expect(row).toBeVisible({ timeout: 60_000 });
+}
+
+async function expectAppShellRendered(page: Page): Promise<void> {
+  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();
+  }
+});

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"]);
+  });
+});

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<string>();
   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<string>();
+  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<string, string[]> | undefined,
 ): Record<string, string[]> {
@@ -134,27 +158,22 @@ export const useSidebarOrderStore = create<SidebarOrderStoreState>()(
       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),
           },
         }));
       },

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<Promise<void>> {
   reads: string[];
 }

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<string, string>();
+
+export function clearAsyncStorageStub(): void {
+  entries.clear();
+}
+
+const AsyncStorageStub = {
+  getItem: (key: string): Promise<string | null> => Promise.resolve(entries.get(key) ?? null),
+  setItem: (key: string, value: string): Promise<void> => {
+    entries.set(key, value);
+    return Promise.resolve();
+  },
+  removeItem: (key: string): Promise<void> => {
+    entries.delete(key);
+    return Promise.resolve();
+  },
+  getAllKeys: (): Promise<string[]> => 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<Array<[string, string | null]>> =>
+    Promise.resolve(keys.map((key) => [key, entries.get(key) ?? null] as [string, string | null])),
+  multiSet: (pairs: ReadonlyArray<readonly [string, string]>): Promise<void> => {
+    for (const [key, value] of pairs) entries.set(key, value);
+    return Promise.resolve();
+  },
+  multiRemove: (keys: readonly string[]): Promise<void> => {
+    for (const key of keys) entries.delete(key);
+    return Promise.resolve();
+  },
+  clear: (): Promise<void> => {
+    entries.clear();
+    return Promise.resolve();
+  },
+};
+
+export default AsyncStorageStub;

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"),