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
172 changes: 129 additions & 43 deletions apps/desktop/src/renderer/commandPalette/ui/LinkTask/LinkTaskFrame.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -6,91 +6,177 @@ import {
} from "@superset/ui/command";
import { toast } from "@superset/ui/sonner";
import { useLiveQuery } from "@tanstack/react-db";
import Fuse from "fuse.js";
import { useMemo } from "react";
import { useDeferredValue, useMemo } from "react";
import {
StatusIcon,
type StatusType,
} from "renderer/routes/_authenticated/_dashboard/tasks/components/TasksView/components/shared/StatusIcon";
import { useHybridSearch } from "renderer/routes/_authenticated/_dashboard/tasks/components/TasksView/hooks/useHybridSearch";
import { useOptimisticCollectionActions } from "renderer/routes/_authenticated/hooks/useOptimisticCollectionActions/useOptimisticCollectionActions";
import { useCollections } from "renderer/routes/_authenticated/providers/CollectionsProvider";
import { useFrameStackStore } from "../../core/frames";
import { useCommandPaletteQuery } from "../CommandPalette/CommandPalette";

const MAX_RESULTS = 25;

// Matches tasks list view ordering: in progress → todo → backlog → done → canceled.
const STATUS_TYPE_ORDER: Record<string, number> = {
started: 0,
unstarted: 1,
backlog: 2,
completed: 3,
canceled: 4,
};

const PRIORITY_ORDER: Record<string, number> = {
urgent: 0,
high: 1,
medium: 2,
low: 3,
none: 4,
};

interface LinkTaskFrameProps {
workspaceId: string;
}

export function LinkTaskFrame({ workspaceId }: LinkTaskFrameProps) {
const collections = useCollections();
const query = useCommandPaletteQuery();
const deferredQuery = useDeferredValue(query);
const setOpen = useFrameStackStore((s) => s.setOpen);
const { v2Workspaces } = useOptimisticCollectionActions();

const { data: tasks = [] } = useLiveQuery(
(q) =>
q.from({ t: collections.tasks }).select(({ t }) => ({
id: t.id,
slug: t.slug,
title: t.title,
description: t.description,
labels: t.labels,
statusId: t.statusId,
priority: t.priority,
externalUrl: t.externalUrl,
updatedAt: t.updatedAt,
})),
[collections.tasks],
);

const fuse = useMemo(
() =>
new Fuse(tasks, {
keys: [
{ name: "slug", weight: 3 },
{ name: "title", weight: 2 },
],
threshold: 0.4,
ignoreLocation: true,
}),
[tasks],
const { data: statuses = [] } = useLiveQuery(
(q) =>
q.from({ s: collections.taskStatuses }).select(({ s }) => ({
id: s.id,
type: s.type,
color: s.color,
position: s.position,
progressPercent: s.progressPercent,
})),
[collections.taskStatuses],
);

const statusMap = useMemo(() => {
const map = new Map<
string,
{
type: StatusType;
color: string;
position: number;
progressPercent: number | null;
}
>();
for (const s of statuses) {
map.set(s.id, {
type: s.type as StatusType,
color: s.color,
position: s.position,
progressPercent: s.progressPercent,
});
}
return map;
}, [statuses]);

const { search } = useHybridSearch(tasks);

const filtered = useMemo(() => {
if (!query) {
if (!deferredQuery) {
return [...tasks]
.sort(
(a, b) =>
new Date(b.updatedAt).getTime() - new Date(a.updatedAt).getTime(),
)
.sort((a, b) => {
const statusA = a.statusId ? statusMap.get(a.statusId) : undefined;
const statusB = b.statusId ? statusMap.get(b.statusId) : undefined;
const typeOrderA =
STATUS_TYPE_ORDER[statusA?.type ?? ""] ?? Number.MAX_SAFE_INTEGER;
const typeOrderB =
STATUS_TYPE_ORDER[statusB?.type ?? ""] ?? Number.MAX_SAFE_INTEGER;
if (typeOrderA !== typeOrderB) return typeOrderA - typeOrderB;
const positionA = statusA?.position ?? Number.MAX_SAFE_INTEGER;
const positionB = statusB?.position ?? Number.MAX_SAFE_INTEGER;
if (positionA !== positionB) return positionA - positionB;
const priorityOrderA =
PRIORITY_ORDER[a.priority] ?? Number.MAX_SAFE_INTEGER;
const priorityOrderB =
PRIORITY_ORDER[b.priority] ?? Number.MAX_SAFE_INTEGER;
return priorityOrderA - priorityOrderB;
})
.slice(0, MAX_RESULTS);
}
return fuse.search(query, { limit: MAX_RESULTS }).map((r) => r.item);
}, [query, fuse, tasks]);
return search(deferredQuery)
.slice(0, MAX_RESULTS)
.map((r) => r.item);
}, [deferredQuery, search, tasks, statusMap]);
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 statusMap is included in the dependency array but is only consulted in the no-query branch of the sort. When deferredQuery is truthy the memo takes the search(deferredQuery) branch, which doesn't touch statusMap, so any status data update will needlessly re-derive filtered and re-render the list mid-keystroke — undercutting the useDeferredValue intent.

Suggested change
}, [deferredQuery, search, tasks, statusMap]);
}, [deferredQuery, search, tasks, statusMap]); // statusMap is only used in the sort path (no query); consider splitting into two memos if status churn causes visible re-renders
Prompt To Fix With AI
This is a comment left during a code review.
Path: apps/desktop/src/renderer/commandPalette/ui/LinkTask/LinkTaskFrame.tsx
Line: 126

Comment:
`statusMap` is included in the dependency array but is only consulted in the no-query branch of the sort. When `deferredQuery` is truthy the memo takes the `search(deferredQuery)` branch, which doesn't touch `statusMap`, so any status data update will needlessly re-derive `filtered` and re-render the list mid-keystroke — undercutting the `useDeferredValue` intent.

```suggestion
	}, [deferredQuery, search, tasks, statusMap]); // statusMap is only used in the sort path (no query); consider splitting into two memos if status churn causes visible re-renders
```

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


const handleSelect = (taskId: string, slug: string) => {
v2Workspaces.updateWorkspace(workspaceId, { taskId });
toast.success(`Linked ${slug} to workspace`);
void linkTaskToWorkspace(taskId, workspaceId);
setOpen(false);
Comment on lines 128 to 131
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.

⚠️ Potential issue | 🟠 Major | ⚡ Quick win

Handle mutation failure before showing success and closing the frame.

updateWorkspace is a remote write path, but the UI always shows success and closes immediately. If the mutation fails (e.g., org/task validation), users get a false success state.

Proposed fix
- const handleSelect = (taskId: string, slug: string) => {
- 	v2Workspaces.updateWorkspace(workspaceId, { taskId });
- 	toast.success(`Linked ${slug} to workspace`);
- 	setOpen(false);
- };
+ const handleSelect = async (taskId: string, slug: string) => {
+ 	try {
+ 		await v2Workspaces.updateWorkspace(workspaceId, { taskId });
+ 		toast.success(`Linked ${slug} to workspace`);
+ 		setOpen(false);
+ 	} catch {
+ 		toast.error(`Failed to link ${slug} to workspace`);
+ 	}
+ };
...
- onSelect={() => handleSelect(task.id, task.slug)}
+ onSelect={() => void handleSelect(task.id, task.slug)}

Also applies to: 147-147

🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@apps/desktop/src/renderer/commandPalette/ui/LinkTask/LinkTaskFrame.tsx`
around lines 128 - 131, The handler handleSelect currently calls
v2Workspaces.updateWorkspace and immediately shows toast.success and calls
setOpen(false) without awaiting or handling errors; change it to await the
updateWorkspace call (or handle its returned promise), wrap it in try/catch,
only call toast.success and setOpen(false) on success, and on failure call
toast.error with the error message (or a friendly message) and do not close the
frame; apply the same pattern to the other similar handler referenced (the one
at the other occurrence around line 147) so both updateWorkspace usages handle
async failures properly.

};

return (
<CommandList className="max-h-[400px]">
<CommandEmpty>No tasks found.</CommandEmpty>
<CommandGroup heading={query ? "Results" : "Recent tasks"}>
{filtered.map((task) => (
<RawCommandItem
key={task.id}
value={`${task.slug} ${task.title}`}
onSelect={() => handleSelect(task.id, task.slug)}
>
<span className="text-xs font-mono text-muted-foreground">
{task.slug}
</span>
<span className="truncate">{task.title}</span>
</RawCommandItem>
))}
</CommandGroup>
{filtered.length > 0 && (
<CommandGroup heading={deferredQuery ? "Results" : "Tasks"}>
{filtered.map((task) => {
const status = task.statusId
? statusMap.get(task.statusId)
: undefined;
return (
<RawCommandItem
key={task.id}
value={`${task.slug} ${task.title}`}
onSelect={() => handleSelect(task.id, task.slug)}
className="group items-start gap-3 rounded-md px-2.5 py-2"
>
<span className="mt-0.5 flex size-4 shrink-0 items-center justify-center">
{status ? (
<StatusIcon
type={status.type}
color={status.color}
progress={status.progressPercent ?? undefined}
/>
) : (
<span className="size-3.5 rounded-full border border-muted-foreground/40" />
)}
</span>
<div className="flex min-w-0 flex-1 flex-col gap-0.5">
<span className="truncate text-sm leading-snug">
{task.title}
</span>
<span className="flex items-center gap-1.5 text-[11px] text-muted-foreground">
<span className="font-mono">{task.slug}</span>
{status ? (
<>
<span aria-hidden>·</span>
<span className="capitalize">{status.type}</span>
</>
) : null}
</span>
</div>
</RawCommandItem>
);
})}
</CommandGroup>
)}
</CommandList>
);
}

async function linkTaskToWorkspace(
taskId: string,
workspaceId: string,
): Promise<void> {
void taskId;
void workspaceId;
}
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@ import { useHotkeyDisplay } from "renderer/hotkeys";
import type { DashboardSidebarWorkspace } from "../../../../types";
import { ChecksList } from "./components/ChecksList";
import { ChecksSummary } from "./components/ChecksSummary";
import { LinkedTaskSection } from "./components/LinkedTaskSection";
import { PullRequestStatusBadge } from "./components/PullRequestStatusBadge";
import { ReviewStatus } from "./components/ReviewStatus";

Expand All @@ -37,6 +38,7 @@ export function DashboardSidebarWorkspaceHoverCardContent({
needsRebase,
behindCount,
createdAt,
taskId,
} = workspace;
const { keys: openPRDisplay } = useHotkeyDisplay("OPEN_PR");
const hasOpenPRShortcut = !(
Expand Down Expand Up @@ -103,6 +105,8 @@ export function DashboardSidebarWorkspaceHoverCardContent({
</span>
</div>

{taskId && <LinkedTaskSection taskId={taskId} />}

{needsRebase && (
<div className="flex items-center gap-2 text-amber-500 text-xs bg-amber-500/10 px-2 py-1.5 rounded-md">
<LuTriangleAlert className="size-3.5 shrink-0" />
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,84 @@
import { eq } from "@tanstack/db";
import { useLiveQuery } from "@tanstack/react-db";
import { Link } from "@tanstack/react-router";
import { LuExternalLink } from "react-icons/lu";
import {
StatusIcon,
type StatusType,
} from "renderer/routes/_authenticated/_dashboard/tasks/components/TasksView/components/shared/StatusIcon";
import { useCollections } from "renderer/routes/_authenticated/providers/CollectionsProvider";

interface LinkedTaskSectionProps {
taskId: string;
}

export function LinkedTaskSection({ taskId }: LinkedTaskSectionProps) {
const collections = useCollections();

const { data: rows = [] } = useLiveQuery(
(q) =>
q
.from({ t: collections.tasks })
.leftJoin({ s: collections.taskStatuses }, ({ t, s }) =>
eq(t.statusId, s.id),
)
.where(({ t }) => eq(t.id, taskId))
.select(({ t, s }) => ({
id: t.id,
slug: t.slug,
title: t.title,
externalUrl: t.externalUrl,
statusType: s?.type ?? null,
statusColor: s?.color ?? null,
statusProgress: s?.progressPercent ?? null,
})),
[collections, taskId],
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 Overly broad query dependency

The live query only reads collections.tasks and collections.taskStatuses, but collections (the whole object) is listed as the dependency. LinkTaskFrame uses the more specific pattern — [collections.tasks] and [collections.taskStatuses] — for exactly this reason. If the collections reference is not perfectly stable, the join will re-run whenever any collection changes (PRs, hosts, members, …), not just on task or status changes. Consider [collections.tasks, collections.taskStatuses, taskId] to stay consistent with the rest of the file.

Prompt To Fix With AI
This is a comment left during a code review.
Path: apps/desktop/src/renderer/routes/_authenticated/_dashboard/components/DashboardSidebar/components/DashboardSidebarWorkspaceItem/components/DashboardSidebarWorkspaceHoverCardContent/components/LinkedTaskSection/LinkedTaskSection.tsx
Line: 35

Comment:
**Overly broad query dependency**

The live query only reads `collections.tasks` and `collections.taskStatuses`, but `collections` (the whole object) is listed as the dependency. `LinkTaskFrame` uses the more specific pattern — `[collections.tasks]` and `[collections.taskStatuses]` — for exactly this reason. If the `collections` reference is not perfectly stable, the join will re-run whenever any collection changes (PRs, hosts, members, …), not just on task or status changes. Consider `[collections.tasks, collections.taskStatuses, taskId]` to stay consistent with the rest of the file.

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

);

const task = rows[0];
if (!task) return null;

return (
<div className="pt-2 border-t border-border space-y-0.5">
<span className="text-[10px] uppercase tracking-wide text-muted-foreground">
Task
</span>
<div className="flex items-center gap-1.5">
<Link
to="/tasks/$taskId"
params={{ taskId: task.id }}
className="group/task flex min-w-0 flex-1 items-center gap-1.5 text-left hover:text-foreground"
title={task.title}
>
<span className="flex size-3.5 shrink-0 items-center justify-center">
{task.statusType ? (
<StatusIcon
type={task.statusType as StatusType}
color={task.statusColor ?? "#9ca3af"}
progress={task.statusProgress ?? undefined}
/>
) : (
<span className="size-3 rounded-full border border-muted-foreground/40" />
)}
</span>
<span className="font-mono text-xs text-muted-foreground shrink-0">
{task.slug}
</span>
<span className="truncate text-xs">{task.title}</span>
</Link>
{task.externalUrl && (
<a
href={task.externalUrl}
target="_blank"
rel="noopener noreferrer"
className="shrink-0 text-muted-foreground hover:text-foreground"
title="Open task externally"
onClick={(e) => e.stopPropagation()}
>
<LuExternalLink className="size-3" />
</a>
Comment on lines +70 to +79
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.

⚠️ Potential issue | 🟡 Minor | ⚡ Quick win

Add an explicit accessible name to the external icon link.

Use aria-label on this icon-only <a> so screen readers have a reliable name.

Suggested patch
 				{task.externalUrl && (
 					<a
 						href={task.externalUrl}
 						target="_blank"
 						rel="noopener noreferrer"
+						aria-label="Open task externally"
 						className="shrink-0 text-muted-foreground hover:text-foreground"
 						title="Open task externally"
 						onClick={(e) => e.stopPropagation()}
 					>
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
<a
href={task.externalUrl}
target="_blank"
rel="noopener noreferrer"
className="shrink-0 text-muted-foreground hover:text-foreground"
title="Open task externally"
onClick={(e) => e.stopPropagation()}
>
<LuExternalLink className="size-3" />
</a>
<a
href={task.externalUrl}
target="_blank"
rel="noopener noreferrer"
aria-label="Open task externally"
className="shrink-0 text-muted-foreground hover:text-foreground"
title="Open task externally"
onClick={(e) => e.stopPropagation()}
>
<LuExternalLink className="size-3" />
</a>
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In
`@apps/desktop/src/renderer/routes/_authenticated/_dashboard/components/DashboardSidebar/components/DashboardSidebarWorkspaceItem/components/DashboardSidebarWorkspaceHoverCardContent/components/LinkedTaskSection/LinkedTaskSection.tsx`
around lines 70 - 79, The external icon-only link in the LinkedTaskSection
component lacks an accessible name; update the anchor element (the <a> wrapping
the LuExternalLink icon in LinkedTaskSection) to include an explicit aria-label
(for example "Open task externally" or include the task title like `Open
{task.title} externally`) so screen readers can announce the purpose; keep the
existing title/onClick/rel attributes and ensure the aria-label value is
meaningful and localized if needed.

)}
</div>
</div>
);
}
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
export { LinkedTaskSection } from "./LinkedTaskSection";
Original file line number Diff line number Diff line change
Expand Up @@ -250,6 +250,7 @@ export function useDashboardSidebarData() {
hostIsOnline: hosts.isOnline,
name: workspaces.name,
branch: workspaces.branch,
taskId: workspaces.taskId,
createdAt: workspaces.createdAt,
updatedAt: workspaces.updatedAt,
tabOrder: sidebarWorkspaces.sidebarState.tabOrder,
Expand Down Expand Up @@ -285,6 +286,7 @@ export function useDashboardSidebarData() {
hostIsOnline: hosts.isOnline,
name: workspaces.name,
branch: workspaces.branch,
taskId: workspaces.taskId,
createdAt: workspaces.createdAt,
updatedAt: workspaces.updatedAt,
tabOrder: MAIN_WORKSPACE_TAB_ORDER,
Expand Down Expand Up @@ -319,6 +321,7 @@ export function useDashboardSidebarData() {
hostIsOnline: host?.isOnline ?? false,
name: cloudRow.name,
branch: cloudRow.branch,
taskId: cloudRow.taskId,
createdAt: cloudRow.createdAt,
updatedAt: cloudRow.updatedAt,
tabOrder:
Expand Down Expand Up @@ -491,6 +494,7 @@ export function useDashboardSidebarData() {
behindCount: null,
createdAt: workspace.createdAt,
updatedAt: workspace.updatedAt,
taskId: workspace.taskId,
};

if (workspace.sectionId) {
Expand Down Expand Up @@ -541,6 +545,7 @@ export function useDashboardSidebarData() {
behindCount: null,
createdAt: new Date(),
updatedAt: new Date(),
taskId: null,
creationStatus: pw.status,
};

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -40,6 +40,7 @@ export interface DashboardSidebarWorkspace {
behindCount: number | null;
createdAt: Date;
updatedAt: Date;
taskId: string | null;
creationStatus?: "preparing" | "generating-branch" | "creating" | "failed";
}

Expand Down
Loading
Loading