fix(web): resolve logical-to-physical key mismatch in project drag reorder (#1904)

Co-authored-by: Julius Marminge <julius0216@outlook.com>

Author: mwolson

apps/web/src/components/Sidebar.tsx

@@ -2696,7 +2696,9 @@ export default function Sidebar() {
       const activeProject = sidebarProjects.find((project) => project.projectKey === active.id);
       const overProject = sidebarProjects.find((project) => project.projectKey === over.id);
       if (!activeProject || !overProject) return;
-      reorderProjects(activeProject.projectKey, overProject.projectKey);
+      const activeMemberKeys = activeProject.memberProjectRefs.map(scopedProjectKey);
+      const overMemberKeys = overProject.memberProjectRefs.map(scopedProjectKey);
+      reorderProjects(activeMemberKeys, overMemberKeys);
     },
     [sidebarProjectSortOrder, reorderProjects, sidebarProjects],
   );

apps/web/src/uiStateStore.test.ts

@@ -58,11 +58,118 @@ describe("uiStateStore pure functions", () => {
       projectOrder: [project1, project2, project3],
     });
 
-    const next = reorderProjects(initialState, project1, project3);
+    const next = reorderProjects(initialState, [project1], [project3]);
 
     expect(next.projectOrder).toEqual([project2, project3, project1]);
   });
 
+  it("reorderProjects is a no-op when dragged key is not in projectOrder", () => {
+    const project1 = ProjectId.make("project-1");
+    const project2 = ProjectId.make("project-2");
+    const initialState = makeUiState({
+      projectOrder: [project1, project2],
+    });
+
+    const next = reorderProjects(initialState, [ProjectId.make("missing")], [project2]);
+
+    expect(next).toBe(initialState);
+  });
+
+  it("reorderProjects moves all member keys of a multi-member group together", () => {
+    const keyALocal = "env-local:proj-a";
+    const keyARemote = "env-remote:proj-a";
+    const keyB = "env-local:proj-b";
+    const keyC = "env-local:proj-c";
+    const initialState = makeUiState({
+      projectOrder: [keyALocal, keyARemote, keyB, keyC],
+    });
+
+    const next = reorderProjects(initialState, [keyALocal, keyARemote], [keyC]);
+
+    expect(next.projectOrder).toEqual([keyB, keyC, keyALocal, keyARemote]);
+  });
+
+  it("reorderProjects handles member keys scattered across projectOrder", () => {
+    const keyALocal = "env-local:proj-a";
+    const keyB = "env-local:proj-b";
+    const keyARemote = "env-remote:proj-a";
+    const keyC = "env-local:proj-c";
+    const initialState = makeUiState({
+      projectOrder: [keyALocal, keyB, keyARemote, keyC],
+    });
+
+    const next = reorderProjects(initialState, [keyALocal, keyARemote], [keyC]);
+
+    expect(next.projectOrder).toEqual([keyB, keyC, keyALocal, keyARemote]);
+  });
+
+  it("reorderProjects places group after target when dragged from before a non-last target", () => {
+    const keyALocal = "env-local:proj-a";
+    const keyARemote = "env-remote:proj-a";
+    const keyB = "env-local:proj-b";
+    const keyC = "env-local:proj-c";
+    const keyD = "env-local:proj-d";
+    const initialState = makeUiState({
+      projectOrder: [keyALocal, keyARemote, keyB, keyC, keyD],
+    });
+
+    const next = reorderProjects(initialState, [keyALocal, keyARemote], [keyC]);
+
+    expect(next.projectOrder).toEqual([keyB, keyC, keyALocal, keyARemote, keyD]);
+  });
+
+  it("reorderProjects places group before target when dragged from after", () => {
+    const keyB = "env-local:proj-b";
+    const keyC = "env-local:proj-c";
+    const keyALocal = "env-local:proj-a";
+    const keyARemote = "env-remote:proj-a";
+    const initialState = makeUiState({
+      projectOrder: [keyB, keyC, keyALocal, keyARemote],
+    });
+
+    const next = reorderProjects(initialState, [keyALocal, keyARemote], [keyB]);
+
+    expect(next.projectOrder).toEqual([keyALocal, keyARemote, keyB, keyC]);
+  });
+
+  it("reorderProjects with multi-member target inserts after first target occurrence", () => {
+    const keyALocal = "env-local:proj-a";
+    const keyARemote = "env-remote:proj-a";
+    const keyBLocal = "env-local:proj-b";
+    const keyBRemote = "env-remote:proj-b";
+    const initialState = makeUiState({
+      projectOrder: [keyALocal, keyARemote, keyBLocal, keyBRemote],
+    });
+
+    const next = reorderProjects(initialState, [keyALocal, keyARemote], [keyBLocal, keyBRemote]);
+
+    // Target members may become non-contiguous; this is fine because the
+    // sidebar groups by logical key using first-occurrence positioning.
+    expect(next.projectOrder).toEqual([keyBLocal, keyALocal, keyARemote, keyBRemote]);
+  });
+
+  it("reorderProjects is a no-op when dragged group equals target group", () => {
+    const key1 = "env-local:proj-a";
+    const key2 = "env-remote:proj-a";
+    const initialState = makeUiState({
+      projectOrder: [key1, key2, "env-local:proj-b"],
+    });
+
+    const next = reorderProjects(initialState, [key1, key2], [key1, key2]);
+
+    expect(next).toBe(initialState);
+  });
+
+  it("reorderProjects is a no-op when dragged keys are not in projectOrder", () => {
+    const initialState = makeUiState({
+      projectOrder: ["env-local:proj-a", "env-local:proj-b"],
+    });
+
+    const next = reorderProjects(initialState, ["env-local:missing"], ["env-local:proj-b"]);
+
+    expect(next).toBe(initialState);
+  });
+
   it("syncProjects preserves current project order during snapshot recovery", () => {
     const project1 = ProjectId.make("project-1");
     const project2 = ProjectId.make("project-2");

apps/web/src/uiStateStore.ts

@@ -482,23 +482,41 @@ export function setProjectExpanded(state: UiState, projectId: string, expanded:
 
 export function reorderProjects(
   state: UiState,
-  draggedProjectId: string,
-  targetProjectId: string,
+  draggedProjectIds: readonly string[],
+  targetProjectIds: readonly string[],
 ): UiState {
-  if (draggedProjectId === targetProjectId) {
+  if (draggedProjectIds.length === 0) {
     return state;
   }
-  const draggedIndex = state.projectOrder.findIndex((projectId) => projectId === draggedProjectId);
-  const targetIndex = state.projectOrder.findIndex((projectId) => projectId === targetProjectId);
-  if (draggedIndex < 0 || targetIndex < 0) {
+  const draggedSet = new Set(draggedProjectIds);
+  const targetSet = new Set(targetProjectIds);
+  if (draggedProjectIds.every((id) => targetSet.has(id))) {
     return state;
   }
+
+  const originalTargetIndex = state.projectOrder.findIndex((id) => targetSet.has(id));
+  if (originalTargetIndex < 0) {
+    return state;
+  }
+
   const projectOrder = [...state.projectOrder];
-  const [draggedProject] = projectOrder.splice(draggedIndex, 1);
-  if (!draggedProject) {
+
+  const removed: string[] = [];
+  let draggedBeforeTarget = 0;
+  for (let i = projectOrder.length - 1; i >= 0; i--) {
+    if (draggedSet.has(projectOrder[i]!)) {
+      removed.unshift(projectOrder.splice(i, 1)[0]!);
+      if (i < originalTargetIndex) {
+        draggedBeforeTarget++;
+      }
+    }
+  }
+  if (removed.length === 0) {
     return state;
   }
-  projectOrder.splice(targetIndex, 0, draggedProject);
+
+  const insertIndex = originalTargetIndex - Math.max(0, draggedBeforeTarget - 1);
+  projectOrder.splice(insertIndex, 0, ...removed);
   return {
     ...state,
     projectOrder,
@@ -514,7 +532,10 @@ interface UiStateStore extends UiState {
   setThreadChangedFilesExpanded: (threadId: string, turnId: string, expanded: boolean) => void;
   toggleProject: (projectId: string) => void;
   setProjectExpanded: (projectId: string, expanded: boolean) => void;
-  reorderProjects: (draggedProjectId: string, targetProjectId: string) => void;
+  reorderProjects: (
+    draggedProjectIds: readonly string[],
+    targetProjectIds: readonly string[],
+  ) => void;
 }
 
 export const useUiStateStore = create<UiStateStore>((set) => ({
@@ -531,8 +552,8 @@ export const useUiStateStore = create<UiStateStore>((set) => ({
   toggleProject: (projectId) => set((state) => toggleProject(state, projectId)),
   setProjectExpanded: (projectId, expanded) =>
     set((state) => setProjectExpanded(state, projectId, expanded)),
-  reorderProjects: (draggedProjectId, targetProjectId) =>
-    set((state) => reorderProjects(state, draggedProjectId, targetProjectId)),
+  reorderProjects: (draggedProjectIds, targetProjectIds) =>
+    set((state) => reorderProjects(state, draggedProjectIds, targetProjectIds)),
 }));
 
 useUiStateStore.subscribe((state) => debouncedPersistState.maybeExecute(state));