Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
46 changes: 46 additions & 0 deletions frontend/src/renderer/components/Sidebar.test.tsx
Original file line number Diff line number Diff line change
@@ -0,0 +1,46 @@
import { render, screen } from "@testing-library/react";
import userEvent from "@testing-library/user-event";
import { describe, expect, it, vi } from "vitest";

// The Sidebar reads selection from the router; stub the three hooks it uses so
// the component renders without a full RouterProvider.
vi.mock("@tanstack/react-router", () => ({
useNavigate: () => vi.fn(),
useParams: () => ({}),
useRouterState: () => "/",
}));

import { TooltipProvider } from "./ui/tooltip";
import { SidebarProvider } from "./ui/sidebar";
import { Sidebar } from "./Sidebar";
import type { WorkspaceSummary } from "../types/workspace";

const workspaces: WorkspaceSummary[] = [{ id: "proj-1", name: "my-app", path: "/p", type: "main", sessions: [] }];

function renderSidebar(onRemoveProject = vi.fn().mockResolvedValue(undefined)) {
render(
<TooltipProvider>
<SidebarProvider>
<Sidebar
daemonStatus={{ state: "running" }}
workspaces={workspaces}
onCreateProject={vi.fn().mockResolvedValue(undefined)}
onRemoveProject={onRemoveProject}
/>
</SidebarProvider>
</TooltipProvider>,
);
return onRemoveProject;
}

describe("Sidebar", () => {
it("removes a project from the row kebab menu", async () => {
const user = userEvent.setup();
const onRemoveProject = renderSidebar();

await user.click(screen.getByRole("button", { name: "Actions for my-app" }));
await user.click(await screen.findByText("Remove project"));

expect(onRemoveProject).toHaveBeenCalledWith("proj-1");
Comment on lines +36 to +44

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 goHome navigation branch isn't exercised by any test. The test stubs useParams to return {}, so activeProjectId is always undefined and the if (selection.activeProjectId === workspace.id) selection.goHome() branch in handleRemove is never reached. A second test case that sets useParams to return { projectId: "proj-1" } would cover the navigation on active-project removal and catch any regression in that branch.

Note: If this suggestion doesn't match your team's coding style, reply to this and let me know. I'll remember it for next time!

});
});
58 changes: 56 additions & 2 deletions frontend/src/renderer/components/Sidebar.tsx
Original file line number Diff line number Diff line change
@@ -1,5 +1,16 @@
import { useNavigate, useParams, useRouterState } from "@tanstack/react-router";
import { ChevronRight, GitPullRequest, Moon, Plus, Search, Settings, Sun, Waypoints } from "lucide-react";
import {
ChevronRight,
GitPullRequest,
Moon,
MoreHorizontal,
Plus,
Search,
Settings,
Sun,
Trash2,
Waypoints,
} from "lucide-react";
import { useState } from "react";
import {
attentionZone,
Expand Down Expand Up @@ -28,6 +39,7 @@ import {
SidebarGroupLabel,
SidebarHeader,
SidebarMenu,
SidebarMenuAction,
SidebarMenuButton,
SidebarMenuItem,
SidebarMenuSub,
Expand All @@ -52,6 +64,7 @@ type SidebarProps = {
workspaceError?: string;
workspaces: WorkspaceSummary[];
onCreateProject: (input: { path: string }) => Promise<void>;
onRemoveProject: (projectId: string) => Promise<void>;
};

// Selection state comes from the URL: which project/session is active is the
Expand Down Expand Up @@ -93,7 +106,7 @@ function SessionDot({ session }: { session: WorkspaceSession }) {
// _shell owns open state (synced to the ui-store) and `collapsible="icon"`
// replaces the old hand-rolled CollapsedRail — the same tree restyles itself
// via group-data-[collapsible=icon] into the 48px letter rail.
export function Sidebar({ daemonStatus, workspaceError, workspaces, onCreateProject }: SidebarProps) {
export function Sidebar({ daemonStatus, workspaceError, workspaces, onCreateProject, onRemoveProject }: SidebarProps) {
const selection = useSelection();
const eventsConnection = useEventsConnection();
const { state } = useSidebar();
Expand Down Expand Up @@ -200,6 +213,7 @@ export function Sidebar({ daemonStatus, workspaceError, workspaces, onCreateProj
expanded={!collapsedIds.has(workspace.id)}
selection={selection}
onToggle={() => toggleCollapsed(workspace.id)}
onRemoveProject={onRemoveProject}
/>
))}
</SidebarMenu>
Expand Down Expand Up @@ -302,13 +316,29 @@ function ProjectItem({
expanded,
selection,
onToggle,
onRemoveProject,
}: {
workspace: WorkspaceSummary;
expanded: boolean;
selection: Selection;
onToggle: () => void;
onRemoveProject: (projectId: string) => Promise<void>;
}) {
const projectActive = selection.activeProjectId === workspace.id && !selection.activeSessionId;

const handleRemove = () => {
void (async () => {
try {
await onRemoveProject(workspace.id);
// The route for a removed project no longer resolves; fall back home.
if (selection.activeProjectId === workspace.id) selection.goHome();
} catch (err) {
// The app has no toast surface yet; the 15s workspace refetch resyncs
// the sidebar, so log and let the row reappear if the delete failed.
console.error("remove project failed", err);
}
})();
};
Comment on lines +329 to +341

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 Navigation fires after the cache update, not atomically with it. onRemoveProject (in _shell.tsx) removes the project from the React Query cache before it returns, which schedules a re-render. selection.goHome() is then called in the next microtask. In React 18's automatic-batching model both state changes are normally merged into one paint, but if the router renders the active project route before the navigation lands, the outlet may briefly see a missing project and throw/404. Moving goHome() into removeProject (in _shell.tsx, where it already owns the cache mutation) and accepting a navigate param would make the two operations atomic from the shell's perspective — but even as-is this is unlikely to surface visually due to batching.

// Live workers only: merged/terminated sessions leave the sidebar and stay
// reachable through the board's Done / Terminated bar (SessionsBoard).
const sessions = workerSessions(workspace.sessions).filter(sessionIsActive);
Expand Down Expand Up @@ -355,6 +385,30 @@ function ProjectItem({
{sessions.length}
</span>
</SidebarMenuButton>
{/* Per-project actions: a kebab that reveals on row hover (replacing the
session count) — the daemon/CLI already support removal, this surfaces
it in the UI. */}
<DropdownMenu>
<DropdownMenuTrigger asChild>
<SidebarMenuAction showOnHover aria-label={`Actions for ${workspace.name}`}>
<MoreHorizontal aria-hidden="true" />
</SidebarMenuAction>
</DropdownMenuTrigger>
Comment on lines +391 to +396

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 The SidebarMenuAction doesn't carry the group-data-[collapsible=icon]:hidden class that the session-count <span> uses to hide itself in the icon/collapsed rail. When the sidebar is collapsed to the 48 px letter rail, hovering the icon tile will still reveal the kebab button — and since DropdownMenuContent opens side="right", the menu will pop out from an already-narrow icon rail, likely overlapping the main content area.

Suggested change
<DropdownMenu>
<DropdownMenuTrigger asChild>
<SidebarMenuAction showOnHover aria-label={`Actions for ${workspace.name}`}>
<MoreHorizontal aria-hidden="true" />
</SidebarMenuAction>
</DropdownMenuTrigger>
<DropdownMenu>
<DropdownMenuTrigger asChild>
<SidebarMenuAction showOnHover aria-label={`Actions for ${workspace.name}`} className="group-data-[collapsible=icon]:hidden">
<MoreHorizontal aria-hidden="true" />
</SidebarMenuAction>
</DropdownMenuTrigger>

Note: If this suggestion doesn't match your team's coding style, reply to this and let me know. I'll remember it for next time!

<DropdownMenuContent side="right" align="start" className="min-w-44">
<DropdownMenuItem onSelect={() => selection.goSettings(workspace.id)}>
<Settings aria-hidden="true" />
Project settings
</DropdownMenuItem>
<DropdownMenuSeparator />
<DropdownMenuItem
className="text-destructive focus:text-destructive [&_svg]:text-destructive"
onSelect={handleRemove}
>
<Trash2 aria-hidden="true" />
Remove project
</DropdownMenuItem>
</DropdownMenuContent>
</DropdownMenu>
{/* project-sidebar__sessions. Divergence from AO (user decision
2026-06-12): no left indent or tree guide line — every sidebar row
(project or worker) spans the same full width. */}
Expand Down
12 changes: 12 additions & 0 deletions frontend/src/renderer/routes/_shell.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -119,6 +119,17 @@ function ShellLayout() {
[navigate, updateWorkspaces],
);

const removeProject = useCallback(
async (projectId: string) => {
const { error } = await apiClient.DELETE("/api/v1/projects/{id}", {
params: { path: { id: projectId } },
});
if (error) throw new Error(apiErrorMessage(error));
updateWorkspaces((current) => current.filter((item) => item.id !== projectId));
},
[updateWorkspaces],
);

useEffect(() => {
document.documentElement.dataset.theme = theme;
document.documentElement.style.colorScheme = theme;
Expand Down Expand Up @@ -174,6 +185,7 @@ function ShellLayout() {
<Sidebar
daemonStatus={daemonStatus}
onCreateProject={createProject}
onRemoveProject={removeProject}
workspaceError={workspaceQuery.isError ? errorMessage(workspaceQuery.error) : undefined}
workspaces={workspaces}
/>
Expand Down
Loading