Skip to content
Merged
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
Original file line number Diff line number Diff line change
Expand Up @@ -214,12 +214,12 @@ export function DashboardSidebarHeader({
horizontal inset — keeps traffic-light alignment matching the
TopBar's 80px pad regardless of parent padding changes. */}
<div
className="drag -mx-2 flex h-8 items-center gap-1.5"
className="drag -mx-2 flex h-8 items-center gap-1.5 pr-2"
style={{ paddingLeft: isMac ? "80px" : "8px" }}
>
<SidebarToggle />
<NavigationControls showHistoryDropdown={false} />
<ResourceConsumption surface="v2" />
<NavigationControls />
<ResourceConsumption surface="v2" className="ml-auto" />
</div>
<OrganizationDropdown variant="expanded" />

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -5,13 +5,7 @@ import { LuArrowLeft, LuArrowRight } from "react-icons/lu";
import { HotkeyLabel, useHotkey } from "renderer/hotkeys";
import { HistoryDropdown } from "./components/HistoryDropdown";

interface NavigationControlsProps {
showHistoryDropdown?: boolean;
}

export function NavigationControls({
showHistoryDropdown = true,
}: NavigationControlsProps = {}) {
export function NavigationControls() {
const router = useRouter();
const location = useLocation();

Expand Down Expand Up @@ -70,7 +64,7 @@ export function NavigationControls({
</TooltipContent>
</Tooltip>

{showHistoryDropdown && <HistoryDropdown />}
<HistoryDropdown />
</div>
);
}
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,9 @@ import { cn } from "@superset/ui/utils";
import { eq } from "@tanstack/db";
import { useLiveQuery } from "@tanstack/react-db";
import { useLocation, useNavigate } from "@tanstack/react-router";
import { LuHistory } from "react-icons/lu";
import { LuCpu, LuGitBranch, LuHistory } from "react-icons/lu";
import { usePresetIcon } from "renderer/assets/app-icons/preset-icons";
import { useIsV2CloudEnabled } from "renderer/hooks/useIsV2CloudEnabled";
import { electronTrpc } from "renderer/lib/electron-trpc";
import {
StatusIcon,
Expand Down Expand Up @@ -75,6 +77,94 @@ function WorkspaceRow({
);
}

function V2WorkspaceRow({
entry,
isCurrent,
v2WorkspaceData,
onSelect,
}: {
entry: RecentlyViewedEntry;
isCurrent: boolean;
v2WorkspaceData: {
id: string;
projectName: string;
branch: string;
}[];
onSelect: () => void;
}) {
const ws = v2WorkspaceData.find((w) => w.id === entry.entityId);

return (
<DropdownMenuItem
className={cn("gap-2.5", isCurrent && "bg-accent/50")}
onSelect={onSelect}
>
<span className="text-muted-foreground text-xs shrink-0 w-20 text-left line-clamp-1">
{ws ? ws.projectName : "Workspace"}
</span>
<span className="flex items-center justify-center w-4 shrink-0">
<LuGitBranch
className="size-3 text-muted-foreground"
strokeWidth={1.5}
/>
</span>
<span
className={cn(
"truncate text-xs font-normal flex-1 min-w-0",
!ws && "text-muted-foreground",
)}
>
{ws ? ws.branch : "Unknown"}
</span>
</DropdownMenuItem>
);
}

function AutomationRow({
entry,
isCurrent,
automationData,
onSelect,
}: {
entry: RecentlyViewedEntry;
isCurrent: boolean;
automationData: {
id: string;
name: string;
agentId: string;
}[];
onSelect: () => void;
}) {
const automation = automationData.find((a) => a.id === entry.entityId);
const presetIcon = usePresetIcon(automation?.agentId ?? "");

return (
<DropdownMenuItem
className={cn("gap-2.5", isCurrent && "bg-accent/50")}
onSelect={onSelect}
>
<span className="text-muted-foreground text-xs shrink-0 w-20 text-left line-clamp-1">
Automation
</span>
<span className="flex items-center justify-center w-4 shrink-0">
{presetIcon ? (
<img src={presetIcon} alt="" className="size-3.5 object-contain" />
) : (
<LuCpu className="size-3 text-muted-foreground" strokeWidth={1.5} />
)}
</span>
<span
className={cn(
"truncate text-xs font-normal flex-1 min-w-0",
!automation && "text-muted-foreground",
)}
>
{automation ? automation.name : "Unknown"}
</span>
</DropdownMenuItem>
);
}

function TaskRow({
entry,
isCurrent,
Expand Down Expand Up @@ -138,6 +228,7 @@ export function HistoryDropdown() {
const recentEntries = useRecentlyViewed(20);
const currentPath = useLocation({ select: (loc) => loc.pathname });
const collections = useCollections();
const isV2CloudEnabled = useIsV2CloudEnabled();

const { data: groups } = electronTrpc.workspaces.getAllGrouped.useQuery();
const workspaceData = (groups ?? []).flatMap((group) =>
Expand All @@ -149,6 +240,34 @@ export function HistoryDropdown() {
})),
);

const { data: v2WorkspaceData } = useLiveQuery(
(q) =>
q
.from({ workspaces: collections.v2Workspaces })
.innerJoin(
{ projects: collections.v2Projects },
({ workspaces, projects }) => eq(workspaces.projectId, projects.id),
)
.select(({ workspaces, projects }) => ({
id: workspaces.id,
projectName: projects.name,
branch: workspaces.branch,
})),
[collections],
);

const { data: automationData } = useLiveQuery(
(q) =>
q
.from({ automations: collections.automations })
.select(({ automations }) => ({
id: automations.id,
name: automations.name,
agentId: automations.agentConfig.id,
})),
[collections],
);

const { data: taskData } = useLiveQuery(
(q) =>
q
Expand All @@ -169,8 +288,17 @@ export function HistoryDropdown() {

const filteredEntries = recentEntries.filter((entry) => {
if (entry.type === "workspace") {
if (isV2CloudEnabled) return false;
return workspaceData.some((w) => w.id === entry.entityId);
}
if (entry.type === "v2-workspace") {
if (!isV2CloudEnabled) return false;
return (v2WorkspaceData ?? []).some((w) => w.id === entry.entityId);
}
if (entry.type === "automation") {
if (!isV2CloudEnabled) return false;
return (automationData ?? []).some((a) => a.id === entry.entityId);
}
Comment on lines +298 to +301
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.

P1 Automation entries not guarded by isV2CloudEnabled

v1-mode and v2-mode workspace entries both have explicit isV2CloudEnabled guards that prevent cross-version navigation, but automation entries have no such guard. If a user has automation visit history from a prior v2 session and then switches to v1 mode, any matching automation record still present in the live DB will pass the automationData.some(...) filter and appear in the dropdown. Clicking one navigates to /automations/${id}, a route that presumably does not exist in v1 mode, likely landing the user on an error or blank screen.

Prompt To Fix With AI
This is a comment left during a code review.
Path: apps/desktop/src/renderer/routes/_authenticated/_dashboard/components/NavigationControls/components/HistoryDropdown/HistoryDropdown.tsx
Line: 298-300

Comment:
**Automation entries not guarded by `isV2CloudEnabled`**

v1-mode and v2-mode workspace entries both have explicit `isV2CloudEnabled` guards that prevent cross-version navigation, but `automation` entries have no such guard. If a user has automation visit history from a prior v2 session and then switches to v1 mode, any matching automation record still present in the live DB will pass the `automationData.some(...)` filter and appear in the dropdown. Clicking one navigates to `/automations/${id}`, a route that presumably does not exist in v1 mode, likely landing the user on an error or blank screen.

How can I resolve this? If you propose a fix, please make it concise.

Comment thread
coderabbitai[bot] marked this conversation as resolved.
return (taskData ?? []).some(
(t) => t.id === entry.entityId || t.slug === entry.entityId,
);
Expand Down Expand Up @@ -211,25 +339,50 @@ export function HistoryDropdown() {
<DropdownMenuContent align="start" className="w-80">
<DropdownMenuLabel>Recently Viewed</DropdownMenuLabel>
<DropdownMenuSeparator />
{filteredEntries.map((entry) =>
entry.type === "task" ? (
<TaskRow
key={entry.path}
entry={entry}
isCurrent={entry.path === currentPath}
taskData={taskData ?? []}
onSelect={() => navigate({ to: entry.path })}
/>
) : (
{filteredEntries.map((entry) => {
if (entry.type === "task") {
return (
<TaskRow
key={entry.path}
entry={entry}
isCurrent={entry.path === currentPath}
taskData={taskData ?? []}
onSelect={() => navigate({ to: entry.path })}
/>
);
}
if (entry.type === "v2-workspace") {
return (
<V2WorkspaceRow
key={entry.path}
entry={entry}
isCurrent={entry.path === currentPath}
v2WorkspaceData={v2WorkspaceData ?? []}
onSelect={() => navigate({ to: entry.path })}
/>
);
}
if (entry.type === "automation") {
return (
<AutomationRow
key={entry.path}
entry={entry}
isCurrent={entry.path === currentPath}
automationData={automationData ?? []}
onSelect={() => navigate({ to: entry.path })}
/>
);
}
return (
<WorkspaceRow
key={entry.path}
entry={entry}
isCurrent={entry.path === currentPath}
workspaceData={workspaceData}
onSelect={() => navigate({ to: entry.path })}
/>
),
)}
);
})}
</DropdownMenuContent>
</DropdownMenu>
);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -4,31 +4,56 @@ import { persistentHistory } from "renderer/lib/persistent-hash-history";

export interface RecentlyViewedEntry {
path: string;
type: "workspace" | "task";
type: "workspace" | "v2-workspace" | "task" | "automation";
entityId: string;
timestamp: number;
}

function pathnameOf(href: string): string {
const queryIndex = href.indexOf("?");
const hashIndex = href.indexOf("#");
const cutoffs = [queryIndex, hashIndex].filter((i) => i >= 0);
return cutoffs.length === 0 ? href : href.substring(0, Math.min(...cutoffs));
}

function parseResourceEntry(entry: {
path: string;
timestamp: number;
}): RecentlyViewedEntry | null {
const wsMatch = entry.path.match(/^\/workspace\/(.+)$/);
const pathname = pathnameOf(entry.path);

const v2WsMatch = pathname.match(/^\/v2-workspace\/([^/]+)/);
if (v2WsMatch?.[1])
return {
path: `/v2-workspace/${v2WsMatch[1]}`,
type: "v2-workspace",
entityId: v2WsMatch[1],
timestamp: entry.timestamp,
};
const wsMatch = pathname.match(/^\/workspace\/([^/]+)/);
if (wsMatch?.[1])
return {
path: entry.path,
path: `/workspace/${wsMatch[1]}`,
type: "workspace",
entityId: wsMatch[1],
timestamp: entry.timestamp,
};
const taskMatch = entry.path.match(/^\/tasks\/(.+)$/);
const taskMatch = pathname.match(/^\/tasks\/([^/]+)/);
if (taskMatch?.[1])
return {
path: entry.path,
path: `/tasks/${taskMatch[1]}`,
type: "task",
entityId: taskMatch[1],
timestamp: entry.timestamp,
};
const automationMatch = pathname.match(/^\/automations\/([^/]+)/);
if (automationMatch?.[1])
return {
path: `/automations/${automationMatch[1]}`,
type: "automation",
entityId: automationMatch[1],
timestamp: entry.timestamp,
};
return null;
}

Expand All @@ -41,10 +66,10 @@ export function useRecentlyViewed(limit = 20): RecentlyViewedEntry[] {

for (let i = allEntries.length - 1; i >= 0; i--) {
const entry = allEntries[i];
if (!entry || seen.has(entry.path)) continue;
if (!entry) continue;
const resource = parseResourceEntry(entry);
if (resource) {
seen.set(entry.path, resource);
if (resource && !seen.has(resource.path)) {
seen.set(resource.path, resource);
}
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -54,8 +54,7 @@ export function TopBar() {
{!sidebarHostsChrome && (
<>
<SidebarToggle />
<NavigationControls showHistoryDropdown={false} />
<ResourceConsumption surface={isV2CloudEnabled ? "v2" : "v1"} />
<NavigationControls />
</>
)}
</div>
Expand Down Expand Up @@ -85,6 +84,9 @@ export function TopBar() {
)}

<div className="flex items-center gap-3 h-full pr-4 shrink-0">
{!sidebarHostsChrome && (
<ResourceConsumption surface={isV2CloudEnabled ? "v2" : "v1"} />
)}
{!isOnline && (
<div className="no-drag flex items-center gap-1.5 text-xs text-muted-foreground bg-muted px-2 py-1 rounded">
<HiOutlineWifi className="size-3.5" />
Expand Down
Loading
Loading