fix: Address code review feedback and fix lint errors

This commit is contained in:
gsxdsm
2026-02-17 00:13:38 -08:00
parent b9653d6338
commit a09a2c76ae
15 changed files with 183 additions and 165 deletions

View File

@@ -1545,7 +1545,8 @@ export function BoardView() {
setSpawnParentFeature(feature);
setShowAddDialog(true);
}}
onDuplicate={handleDuplicateFeature}
onDuplicate={(feature) => handleDuplicateFeature(feature, false)}
onDuplicateAsChild={(feature) => handleDuplicateFeature(feature, true)}
featuresWithContext={featuresWithContext}
runningAutoTasks={runningAutoTasksAllWorktrees}
onArchiveAllVerified={() => setShowArchiveAllVerifiedDialog(true)}

View File

@@ -31,6 +31,49 @@ import { formatModelName, DEFAULT_MODEL } from '@/lib/agent-context-parser';
import { DeleteConfirmDialog } from '@/components/ui/delete-confirm-dialog';
import { getProviderIconForModel } from '@/components/ui/provider-icon';
function DuplicateMenuItems({
onDuplicate,
onDuplicateAsChild,
}: {
onDuplicate?: () => void;
onDuplicateAsChild?: () => void;
}) {
if (!onDuplicate) return null;
return (
<DropdownMenuSub>
<div className="flex items-center">
<DropdownMenuItem
onClick={(e) => {
e.stopPropagation();
onDuplicate();
}}
className="text-xs flex-1 pr-0 rounded-r-none"
>
<Copy className="w-3 h-3 mr-2" />
Duplicate
</DropdownMenuItem>
{onDuplicateAsChild && (
<DropdownMenuSubTrigger className="text-xs px-1 rounded-l-none border-l border-border/30 h-8" />
)}
</div>
{onDuplicateAsChild && (
<DropdownMenuSubContent>
<DropdownMenuItem
onClick={(e) => {
e.stopPropagation();
onDuplicateAsChild();
}}
className="text-xs"
>
<GitFork className="w-3 h-3 mr-2" />
Duplicate as Child
</DropdownMenuItem>
</DropdownMenuSubContent>
)}
</DropdownMenuSub>
);
}
interface CardHeaderProps {
feature: Feature;
isDraggable: boolean;
@@ -122,39 +165,10 @@ export const CardHeaderSection = memo(function CardHeaderSection({
<GitFork className="w-3 h-3 mr-2" />
Spawn Sub-Task
</DropdownMenuItem>
{onDuplicate && (
<DropdownMenuSub>
<div className="flex items-center">
<DropdownMenuItem
onClick={(e) => {
e.stopPropagation();
onDuplicate();
}}
className="text-xs flex-1 pr-0 rounded-r-none"
>
<Copy className="w-3 h-3 mr-2" />
Duplicate
</DropdownMenuItem>
{onDuplicateAsChild && (
<DropdownMenuSubTrigger className="text-xs px-1 rounded-l-none border-l border-border/30 h-8" />
)}
</div>
{onDuplicateAsChild && (
<DropdownMenuSubContent>
<DropdownMenuItem
onClick={(e) => {
e.stopPropagation();
onDuplicateAsChild();
}}
className="text-xs"
>
<GitFork className="w-3 h-3 mr-2" />
Duplicate as Child
</DropdownMenuItem>
</DropdownMenuSubContent>
)}
</DropdownMenuSub>
)}
<DuplicateMenuItems
onDuplicate={onDuplicate}
onDuplicateAsChild={onDuplicateAsChild}
/>
{/* Model info in dropdown */}
{(() => {
const ProviderIcon = getProviderIconForModel(feature.model);
@@ -217,39 +231,10 @@ export const CardHeaderSection = memo(function CardHeaderSection({
</Button>
</DropdownMenuTrigger>
<DropdownMenuContent align="end" className="w-40">
{onDuplicate && (
<DropdownMenuSub>
<div className="flex items-center">
<DropdownMenuItem
onClick={(e) => {
e.stopPropagation();
onDuplicate();
}}
className="text-xs flex-1 pr-0 rounded-r-none"
>
<Copy className="w-3 h-3 mr-2" />
Duplicate
</DropdownMenuItem>
{onDuplicateAsChild && (
<DropdownMenuSubTrigger className="text-xs px-1 rounded-l-none border-l border-border/30 h-8" />
)}
</div>
{onDuplicateAsChild && (
<DropdownMenuSubContent>
<DropdownMenuItem
onClick={(e) => {
e.stopPropagation();
onDuplicateAsChild();
}}
className="text-xs"
>
<GitFork className="w-3 h-3 mr-2" />
Duplicate as Child
</DropdownMenuItem>
</DropdownMenuSubContent>
)}
</DropdownMenuSub>
)}
<DuplicateMenuItems
onDuplicate={onDuplicate}
onDuplicateAsChild={onDuplicateAsChild}
/>
</DropdownMenuContent>
</DropdownMenu>
</div>
@@ -337,39 +322,10 @@ export const CardHeaderSection = memo(function CardHeaderSection({
<GitFork className="w-3 h-3 mr-2" />
Spawn Sub-Task
</DropdownMenuItem>
{onDuplicate && (
<DropdownMenuSub>
<div className="flex items-center">
<DropdownMenuItem
onClick={(e) => {
e.stopPropagation();
onDuplicate();
}}
className="text-xs flex-1 pr-0 rounded-r-none"
>
<Copy className="w-3 h-3 mr-2" />
Duplicate
</DropdownMenuItem>
{onDuplicateAsChild && (
<DropdownMenuSubTrigger className="text-xs px-1 rounded-l-none border-l border-border/30 h-8" />
)}
</div>
{onDuplicateAsChild && (
<DropdownMenuSubContent>
<DropdownMenuItem
onClick={(e) => {
e.stopPropagation();
onDuplicateAsChild();
}}
className="text-xs"
>
<GitFork className="w-3 h-3 mr-2" />
Duplicate as Child
</DropdownMenuItem>
</DropdownMenuSubContent>
)}
</DropdownMenuSub>
)}
<DuplicateMenuItems
onDuplicate={onDuplicate}
onDuplicateAsChild={onDuplicateAsChild}
/>
</DropdownMenuContent>
</DropdownMenu>
</div>
@@ -440,39 +396,10 @@ export const CardHeaderSection = memo(function CardHeaderSection({
<GitFork className="w-3 h-3 mr-2" />
Spawn Sub-Task
</DropdownMenuItem>
{onDuplicate && (
<DropdownMenuSub>
<div className="flex items-center">
<DropdownMenuItem
onClick={(e) => {
e.stopPropagation();
onDuplicate();
}}
className="text-xs flex-1 pr-0 rounded-r-none"
>
<Copy className="w-3 h-3 mr-2" />
Duplicate
</DropdownMenuItem>
{onDuplicateAsChild && (
<DropdownMenuSubTrigger className="text-xs px-1 rounded-l-none border-l border-border/30 h-8" />
)}
</div>
{onDuplicateAsChild && (
<DropdownMenuSubContent>
<DropdownMenuItem
onClick={(e) => {
e.stopPropagation();
onDuplicateAsChild();
}}
className="text-xs"
>
<GitFork className="w-3 h-3 mr-2" />
Duplicate as Child
</DropdownMenuItem>
</DropdownMenuSubContent>
)}
</DropdownMenuSub>
)}
<DuplicateMenuItems
onDuplicate={onDuplicate}
onDuplicateAsChild={onDuplicateAsChild}
/>
{/* Model info in dropdown */}
{(() => {
const ProviderIcon = getProviderIconForModel(feature.model);

View File

@@ -85,6 +85,11 @@ export function useBoardPersistence({ currentProject }: UseBoardPersistenceProps
throw new Error('Features API not available');
}
// Capture previous cache snapshot for synchronous rollback on error
const previousFeatures = queryClient.getQueryData<Feature[]>(
queryKeys.features.all(currentProject.path)
);
// Optimistically add to React Query cache for immediate board refresh
queryClient.setQueryData<Feature[]>(
queryKeys.features.all(currentProject.path),
@@ -95,6 +100,16 @@ export function useBoardPersistence({ currentProject }: UseBoardPersistenceProps
const result = await api.features.create(currentProject.path, feature as ApiFeature);
if (result.success && result.feature) {
updateFeature(result.feature.id, result.feature as Partial<Feature>);
// Update cache with server-confirmed feature before invalidating
queryClient.setQueryData<Feature[]>(
queryKeys.features.all(currentProject.path),
(features) => {
if (!features) return features;
return features.map((f) =>
f.id === result.feature!.id ? { ...f, ...(result.feature as Feature) } : f
);
}
);
} else if (!result.success) {
throw new Error(result.error || 'Failed to create feature on server');
}
@@ -104,7 +119,10 @@ export function useBoardPersistence({ currentProject }: UseBoardPersistenceProps
});
} catch (error) {
logger.error('Failed to persist feature creation:', error);
// Rollback optimistic update on error
// Rollback optimistic update synchronously on error
if (previousFeatures) {
queryClient.setQueryData(queryKeys.features.all(currentProject.path), previousFeatures);
}
queryClient.invalidateQueries({
queryKey: queryKeys.features.all(currentProject.path),
});
@@ -131,7 +149,6 @@ export function useBoardPersistence({ currentProject }: UseBoardPersistenceProps
try {
const api = getElectronAPI();
if (!api.features) {
logger.error('Features API not available');
// Rollback optimistic deletion since we can't persist
if (previousFeatures) {
queryClient.setQueryData(queryKeys.features.all(currentProject.path), previousFeatures);
@@ -139,7 +156,7 @@ export function useBoardPersistence({ currentProject }: UseBoardPersistenceProps
queryClient.invalidateQueries({
queryKey: queryKeys.features.all(currentProject.path),
});
return;
throw new Error('Features API not available');
}
await api.features.delete(currentProject.path, featureId);

View File

@@ -46,7 +46,8 @@ interface KanbanBoardProps {
onViewPlan: (feature: Feature) => void;
onApprovePlan: (feature: Feature) => void;
onSpawnTask?: (feature: Feature) => void;
onDuplicate?: (feature: Feature, asChild: boolean) => void;
onDuplicate?: (feature: Feature) => void;
onDuplicateAsChild?: (feature: Feature) => void;
featuresWithContext: Set<string>;
runningAutoTasks: string[];
onArchiveAllVerified: () => void;
@@ -284,6 +285,7 @@ export function KanbanBoard({
onApprovePlan,
onSpawnTask,
onDuplicate,
onDuplicateAsChild,
featuresWithContext,
runningAutoTasks,
onArchiveAllVerified,
@@ -571,8 +573,8 @@ export function KanbanBoard({
onViewPlan={() => onViewPlan(feature)}
onApprovePlan={() => onApprovePlan(feature)}
onSpawnTask={() => onSpawnTask?.(feature)}
onDuplicate={() => onDuplicate?.(feature, false)}
onDuplicateAsChild={() => onDuplicate?.(feature, true)}
onDuplicate={() => onDuplicate?.(feature)}
onDuplicateAsChild={() => onDuplicateAsChild?.(feature)}
hasContext={featuresWithContext.has(feature.id)}
isCurrentAutoTask={runningAutoTasks.includes(feature.id)}
shortcutKey={shortcutKey}
@@ -615,8 +617,8 @@ export function KanbanBoard({
onViewPlan={() => onViewPlan(feature)}
onApprovePlan={() => onApprovePlan(feature)}
onSpawnTask={() => onSpawnTask?.(feature)}
onDuplicate={() => onDuplicate?.(feature, false)}
onDuplicateAsChild={() => onDuplicate?.(feature, true)}
onDuplicate={() => onDuplicate?.(feature)}
onDuplicateAsChild={() => onDuplicateAsChild?.(feature)}
hasContext={featuresWithContext.has(feature.id)}
isCurrentAutoTask={runningAutoTasks.includes(feature.id)}
shortcutKey={shortcutKey}

View File

@@ -32,7 +32,6 @@ function featureToInternal(feature: Feature): FeatureWithId {
}
function internalToFeature(internal: FeatureWithId): Feature {
// eslint-disable-next-line @typescript-eslint/no-unused-vars
const { _id, _locationIds, ...feature } = internal;
return feature;
}

View File

@@ -27,7 +27,6 @@ function phaseToInternal(phase: RoadmapPhase): PhaseWithId {
}
function internalToPhase(internal: PhaseWithId): RoadmapPhase {
// eslint-disable-next-line @typescript-eslint/no-unused-vars
const { _id, ...phase } = internal;
return phase;
}