diff --git a/docs/AGENTS.md b/docs/AGENTS.md index 89edc0d3c..e9b486bb5 100644 --- a/docs/AGENTS.md +++ b/docs/AGENTS.md @@ -190,9 +190,8 @@ This project uses **Make** as the primary build orchestrator. See `Makefile` for ### General Testing Guidelines - Always run `make typecheck` after making changes to verify types (checks both main and renderer) -- **Unit tests should be colocated with their business logic** - Place unit test files (\*.test.ts) in the same directory as the code they test (e.g., `aiService.test.ts` next to `aiService.ts`) +- **⚠️ CRITICAL: Unit tests MUST be colocated with the code they test** - Place `*.test.ts` files in the same directory as the implementation file (e.g., `src/utils/foo.test.ts` next to `src/utils/foo.ts`). Tests in `./tests/` are ONLY for integration/E2E tests that require complex setup. - **Don't test simple mapping operations** - If the test just verifies the code does what it obviously does from reading it, skip the test. -- E2E and integration tests may live in `./tests/` directory, but unit tests must be colocated - Strive to decompose complex logic away from the components and into `.src/utils/` - utils should be either pure functions or easily isolated (e.g. if they operate on the FS they accept a path). Testing them should not require complex mocks or setup. diff --git a/src/components/ProjectSidebar.tsx b/src/components/ProjectSidebar.tsx index 630170ee5..aa23da3a0 100644 --- a/src/components/ProjectSidebar.tsx +++ b/src/components/ProjectSidebar.tsx @@ -665,6 +665,12 @@ const ProjectSidebarInner: React.FC = ({ // Normalize order when the set of projects changes (not on every parent render) useEffect(() => { + // Skip normalization if projects haven't loaded yet (empty Map on initial render) + // This prevents clearing projectOrder before projects load from backend + if (projects.size === 0) { + return; + } + const normalized = normalizeOrder(projectOrder, projects); if ( normalized.length !== projectOrder.length || diff --git a/src/utils/projectOrdering.test.ts b/src/utils/projectOrdering.test.ts new file mode 100644 index 000000000..6012cf8db --- /dev/null +++ b/src/utils/projectOrdering.test.ts @@ -0,0 +1,142 @@ +import { describe, it, expect } from "@jest/globals"; +import { + sortProjectsByOrder, + reorderProjects, + normalizeOrder, + equalOrders, +} from "./projectOrdering"; +import type { ProjectConfig } from "@/config"; + +describe("projectOrdering", () => { + const createProjects = (paths: string[]): Map => { + const map = new Map(); + for (const p of paths) { + map.set(p, { workspaces: [] }); + } + return map; + }; + + describe("sortProjectsByOrder", () => { + it("returns natural order when order array is empty", () => { + const projects = createProjects(["/a", "/c", "/b"]); + const result = sortProjectsByOrder(projects, []); + expect(result.map(([p]) => p)).toEqual(["/a", "/c", "/b"]); + }); + + it("sorts projects according to order array", () => { + const projects = createProjects(["/a", "/b", "/c"]); + const result = sortProjectsByOrder(projects, ["/c", "/a", "/b"]); + expect(result.map(([p]) => p)).toEqual(["/c", "/a", "/b"]); + }); + + it("puts unknown projects at the end in natural order", () => { + const projects = createProjects(["/a", "/b", "/c", "/d"]); + const result = sortProjectsByOrder(projects, ["/c", "/a"]); + // /c and /a are ordered, /b and /d are unknown and should appear in natural order + expect(result.map(([p]) => p)).toEqual(["/c", "/a", "/b", "/d"]); + }); + }); + + describe("reorderProjects", () => { + it("moves dragged project to target position", () => { + const projects = createProjects(["/a", "/b", "/c", "/d"]); + const currentOrder = ["/a", "/b", "/c", "/d"]; + // Drag /d onto /b (move /d to position 1) + const result = reorderProjects(currentOrder, projects, "/d", "/b"); + expect(result).toEqual(["/a", "/d", "/b", "/c"]); + }); + + it("returns current order if dragged or target not found", () => { + const projects = createProjects(["/a", "/b", "/c"]); + const currentOrder = ["/a", "/b", "/c"]; + const result = reorderProjects(currentOrder, projects, "/x", "/b"); + expect(result).toEqual(["/a", "/b", "/c"]); + }); + + it("returns current order if dragged === target", () => { + const projects = createProjects(["/a", "/b", "/c"]); + const currentOrder = ["/a", "/b", "/c"]; + const result = reorderProjects(currentOrder, projects, "/b", "/b"); + expect(result).toEqual(["/a", "/b", "/c"]); + }); + }); + + describe("normalizeOrder", () => { + it("removes paths that no longer exist", () => { + const projects = createProjects(["/a", "/b"]); + const order = ["/a", "/b", "/c", "/d"]; + const result = normalizeOrder(order, projects); + expect(result).toEqual(["/a", "/b"]); + }); + + it("appends new projects to the end", () => { + const projects = createProjects(["/a", "/b", "/c", "/d"]); + const order = ["/b", "/a"]; + const result = normalizeOrder(order, projects); + expect(result).toEqual(["/b", "/a", "/c", "/d"]); + }); + + it("preserves order of existing projects", () => { + const projects = createProjects(["/a", "/b", "/c"]); + const order = ["/c", "/a", "/b"]; + const result = normalizeOrder(order, projects); + expect(result).toEqual(["/c", "/a", "/b"]); + }); + }); + + describe("equalOrders", () => { + it("returns true for identical arrays", () => { + const a = ["/a", "/b", "/c"]; + const b = ["/a", "/b", "/c"]; + expect(equalOrders(a, b)).toBe(true); + }); + + it("returns false for arrays with different lengths", () => { + const a = ["/a", "/b"]; + const b = ["/a", "/b", "/c"]; + expect(equalOrders(a, b)).toBe(false); + }); + + it("returns false for arrays with different order", () => { + const a = ["/a", "/b", "/c"]; + const b = ["/a", "/c", "/b"]; + expect(equalOrders(a, b)).toBe(false); + }); + + it("returns true for same reference", () => { + const a = ["/a", "/b", "/c"]; + expect(equalOrders(a, a)).toBe(true); + }); + }); + + describe("Bug fix: empty projects Map on initial load", () => { + it("returns empty array when projects Map is empty", () => { + // This documents the bug scenario: + // 1. localStorage has projectOrder = ["/a", "/b", "/c"] + // 2. Projects haven't loaded yet, so projects = new Map() + // 3. If normalization runs, it would clear the order + const emptyProjects = createProjects([]); + const order = ["/a", "/b", "/c"]; + const result = normalizeOrder(order, emptyProjects); + + // normalizeOrder returns [] when projects is empty + expect(result).toEqual([]); + + // Fix: ProjectSidebar.tsx skips normalization when projects.size === 0 + // This prevents clearing the order during initial component mount + }); + + it("normalizes correctly after projects load", () => { + // After projects load, normalization should work normally: + // 1. projectOrder is still ["/a", "/b", "/c"] from localStorage + // 2. Projects are now loaded with an additional project ["/d"] + // 3. Normalization should append the new project + const projectOrder = ["/a", "/b", "/c"]; + const loadedProjects = createProjects(["/a", "/b", "/c", "/d"]); + + const result = normalizeOrder(projectOrder, loadedProjects); + + expect(result).toEqual(["/a", "/b", "/c", "/d"]); + }); + }); +});