diff --git a/apps/ui/src/components/views/board-view.tsx b/apps/ui/src/components/views/board-view.tsx index 66fa50d0..f30ec20f 100644 --- a/apps/ui/src/components/views/board-view.tsx +++ b/apps/ui/src/components/views/board-view.tsx @@ -1057,6 +1057,7 @@ export function BoardView() { setSpawnParentFeature(feature); setShowAddDialog(true); }} + onDeleteTask={(feature) => handleDeleteFeature(feature.id)} /> )} diff --git a/apps/ui/src/components/views/board-view/dialogs/add-feature-dialog.tsx b/apps/ui/src/components/views/board-view/dialogs/add-feature-dialog.tsx index 86f45505..a5eea2c5 100644 --- a/apps/ui/src/components/views/board-view/dialogs/add-feature-dialog.tsx +++ b/apps/ui/src/components/views/board-view/dialogs/add-feature-dialog.tsx @@ -169,9 +169,8 @@ export function AddFeatureDialog({ if (parentFeature) { const ancestorList = getAncestors(parentFeature, allFeatures); setAncestors(ancestorList); - // Select all ancestors by default (including parent) - const allIds = new Set([parentFeature.id, ...ancestorList.map((a) => a.id)]); - setSelectedAncestorIds(allIds); + // Only select parent by default - ancestors are optional context + setSelectedAncestorIds(new Set([parentFeature.id])); } else { setAncestors([]); setSelectedAncestorIds(new Set()); diff --git a/apps/ui/src/components/views/board-view/shared/ancestor-context-section.tsx b/apps/ui/src/components/views/board-view/shared/ancestor-context-section.tsx index bbdeebda..c0a40293 100644 --- a/apps/ui/src/components/views/board-view/shared/ancestor-context-section.tsx +++ b/apps/ui/src/components/views/board-view/shared/ancestor-context-section.tsx @@ -2,7 +2,7 @@ import { useState } from 'react'; import { Checkbox } from '@/components/ui/checkbox'; import { Label } from '@/components/ui/label'; import { Button } from '@/components/ui/button'; -import { ChevronDown, ChevronRight, Users } from 'lucide-react'; +import { ChevronDown, ChevronRight, Users, CheckCircle2 } from 'lucide-react'; import { cn } from '@/lib/utils'; import type { AncestorContext } from '@automaker/dependency-resolver'; import { Collapsible, CollapsibleContent, CollapsibleTrigger } from '@/components/ui/collapsible'; @@ -102,7 +102,8 @@ export function AncestorContextSection({

- Select ancestors to include their context in the new task's prompt. + The parent task context will be included to help the AI understand the background. + Additional ancestors can optionally be included for more context.

@@ -122,7 +123,13 @@ export function AncestorContextSection({
@@ -156,10 +163,13 @@ export function AncestorContextSection({ className="text-sm font-medium cursor-pointer truncate flex-1" > {displayTitle} - {item.isParent && ( - (Parent) - )} + {item.isParent && ( + + + Completed Parent + + )}
diff --git a/apps/ui/src/components/views/graph-view/components/dependency-edge.tsx b/apps/ui/src/components/views/graph-view/components/dependency-edge.tsx index cbf3cdb9..95f72870 100644 --- a/apps/ui/src/components/views/graph-view/components/dependency-edge.tsx +++ b/apps/ui/src/components/views/graph-view/components/dependency-edge.tsx @@ -1,14 +1,16 @@ -import { memo } from 'react'; +import { memo, useState } from 'react'; import { BaseEdge, getBezierPath, EdgeLabelRenderer } from '@xyflow/react'; import type { EdgeProps } from '@xyflow/react'; import { cn } from '@/lib/utils'; import { Feature } from '@/store/app-store'; +import { Trash2 } from 'lucide-react'; export interface DependencyEdgeData { sourceStatus: Feature['status']; targetStatus: Feature['status']; isHighlighted?: boolean; isDimmed?: boolean; + onDeleteDependency?: (sourceId: string, targetId: string) => void; } const getEdgeColor = (sourceStatus?: Feature['status'], targetStatus?: Feature['status']) => { @@ -31,6 +33,8 @@ const getEdgeColor = (sourceStatus?: Feature['status'], targetStatus?: Feature[' export const DependencyEdge = memo(function DependencyEdge(props: EdgeProps) { const { id, + source, + target, sourceX, sourceY, targetX, @@ -42,6 +46,7 @@ export const DependencyEdge = memo(function DependencyEdge(props: EdgeProps) { animated, } = props; + const [isHovered, setIsHovered] = useState(false); const edgeData = data as DependencyEdgeData | undefined; const [edgePath, labelX, labelY] = getBezierPath({ @@ -67,14 +72,30 @@ export const DependencyEdge = memo(function DependencyEdge(props: EdgeProps) { edgeData?.sourceStatus === 'completed' || edgeData?.sourceStatus === 'verified'; const isInProgress = edgeData?.targetStatus === 'in_progress'; + const handleDelete = (e: React.MouseEvent) => { + e.stopPropagation(); + edgeData?.onDeleteDependency?.(source, target); + }; + return ( <> + {/* Invisible wider path for hover detection */} + setIsHovered(true)} + onMouseLeave={() => setIsHovered(false)} + style={{ cursor: 'pointer' }} + /> + {/* Background edge for better visibility */} + {/* Delete button on hover or select */} + {(isHovered || selected) && edgeData?.onDeleteDependency && ( + +
setIsHovered(true)} + onMouseLeave={() => setIsHovered(false)} + > + +
+
+ )} + {/* Animated particles for in-progress edges */} - {animated && ( + {animated && !isHovered && (
{/* Target handle (left side - receives dependencies) */} Spawn Sub-Task + {!data.isRunning && ( + { + e.stopPropagation(); + data.onDeleteTask?.(); + }} + > + + Delete Task + + )}
@@ -336,8 +351,10 @@ export const TaskNode = memo(function TaskNode({ data, selected }: TaskNodeProps {/* Source handle (right side - provides to dependents) */} >(new Set()); + + // Update nodes/edges when features change, but preserve user positions useEffect(() => { - setNodes(layoutedNodes); - setEdges(layoutedEdges); + const currentNodeIds = new Set(layoutedNodes.map((n) => n.id)); + const isInitialRender = !hasInitialLayout.current; + + // Check if there are new nodes that need layout + const hasNewNodes = layoutedNodes.some((n) => !prevNodeIds.current.has(n.id)); + + if (isInitialRender) { + // Apply full layout for initial render + setNodes(layoutedNodes); + setEdges(layoutedEdges); + hasInitialLayout.current = true; + } else if (hasNewNodes) { + // New nodes added - need to re-layout but try to preserve existing positions + setNodes((currentNodes) => { + const positionMap = new Map(currentNodes.map((n) => [n.id, n.position])); + return layoutedNodes.map((node) => ({ + ...node, + position: positionMap.get(node.id) || node.position, + })); + }); + setEdges(layoutedEdges); + } else { + // No new nodes - just update data without changing positions + setNodes((currentNodes) => { + const positionMap = new Map(currentNodes.map((n) => [n.id, n.position])); + // Filter to only include nodes that still exist + const existingNodeIds = new Set(layoutedNodes.map((n) => n.id)); + + return layoutedNodes.map((node) => ({ + ...node, + position: positionMap.get(node.id) || node.position, + })); + }); + // Update edges without triggering re-render of nodes + setEdges(layoutedEdges); + } + + // Update prev node IDs for next comparison + prevNodeIds.current = currentNodeIds; }, [layoutedNodes, layoutedEdges, setNodes, setEdges]); // Handle layout direction change @@ -154,6 +196,16 @@ function GraphCanvasInner({ [onCreateDependency] ); + // Allow any connection between different nodes + const isValidConnection = useCallback( + (connection: Connection | { source: string; target: string }) => { + // Don't allow self-connections + if (connection.source === connection.target) return false; + return true; + }, + [] + ); + // MiniMap node color based on status const minimapNodeColor = useCallback((node: Node) => { const data = node.data as TaskNodeData | undefined; @@ -182,6 +234,7 @@ function GraphCanvasInner({ onEdgesChange={onEdgesChange} onNodeDoubleClick={handleNodeDoubleClick} onConnect={handleConnect} + isValidConnection={isValidConnection} nodeTypes={nodeTypes} edgeTypes={edgeTypes} fitView diff --git a/apps/ui/src/components/views/graph-view/graph-view.tsx b/apps/ui/src/components/views/graph-view/graph-view.tsx index 7435348f..b3ca5e3c 100644 --- a/apps/ui/src/components/views/graph-view/graph-view.tsx +++ b/apps/ui/src/components/views/graph-view/graph-view.tsx @@ -21,6 +21,7 @@ interface GraphViewProps { onResumeTask?: (feature: Feature) => void; onUpdateFeature?: (featureId: string, updates: Partial) => void; onSpawnTask?: (feature: Feature) => void; + onDeleteTask?: (feature: Feature) => void; } export function GraphView({ @@ -38,6 +39,7 @@ export function GraphView({ onResumeTask, onUpdateFeature, onSpawnTask, + onDeleteTask, }: GraphViewProps) { const { currentProject } = useAppStore(); @@ -83,6 +85,34 @@ export function GraphView({ // Handle creating a dependency via edge connection const handleCreateDependency = useCallback( async (sourceId: string, targetId: string): Promise => { + const sourceFeature = features.find((f) => f.id === sourceId); + const targetFeature = features.find((f) => f.id === targetId); + + // Debug logging + console.log('[Dependency Check] ==========================='); + console.log('[Dependency Check] Source (prerequisite):', { + id: sourceId, + title: sourceFeature?.title || sourceFeature?.description?.slice(0, 50), + dependencies: sourceFeature?.dependencies, + }); + console.log('[Dependency Check] Target (will depend on source):', { + id: targetId, + title: targetFeature?.title || targetFeature?.description?.slice(0, 50), + dependencies: targetFeature?.dependencies, + }); + console.log( + '[Dependency Check] Action:', + `${targetFeature?.title || targetId} will depend on ${sourceFeature?.title || sourceId}` + ); + console.log( + '[Dependency Check] All features:', + features.map((f) => ({ + id: f.id, + title: f.title || f.description?.slice(0, 30), + deps: f.dependencies, + })) + ); + // Prevent self-dependency if (sourceId === targetId) { toast.error('A task cannot depend on itself'); @@ -96,7 +126,18 @@ export function GraphView({ } // Check for circular dependency - if (wouldCreateCircularDependency(features, sourceId, targetId)) { + // This checks: if we make targetId depend on sourceId, would it create a cycle? + // A cycle would occur if sourceId already depends on targetId (transitively) + console.log('[Cycle Check] Checking if adding dependency would create cycle...'); + console.log( + '[Cycle Check] Would create cycle if:', + sourceFeature?.title || sourceId, + 'already depends on', + targetFeature?.title || targetId + ); + const wouldCycle = wouldCreateCircularDependency(features, sourceId, targetId); + console.log('[Cycle Check] Result:', wouldCycle ? 'WOULD CREATE CYCLE' : 'Safe to add'); + if (wouldCycle) { toast.error('Cannot create circular dependency', { description: 'This would create a dependency cycle', }); @@ -104,13 +145,12 @@ export function GraphView({ } // Get target feature and update its dependencies - const targetFeature = features.find((f) => f.id === targetId); if (!targetFeature) { toast.error('Target task not found'); return false; } - const currentDeps = targetFeature.dependencies || []; + const currentDeps = (targetFeature.dependencies as string[] | undefined) || []; // Add the dependency onUpdateFeature?.(targetId, { @@ -162,8 +202,38 @@ export function GraphView({ onSpawnTask?.(feature); } }, + onDeleteTask: (featureId: string) => { + const feature = features.find((f) => f.id === featureId); + if (feature) { + onDeleteTask?.(feature); + } + }, + onDeleteDependency: (sourceId: string, targetId: string) => { + // Find the target feature and remove the source from its dependencies + const targetFeature = features.find((f) => f.id === targetId); + if (!targetFeature) return; + + const currentDeps = (targetFeature.dependencies as string[] | undefined) || []; + const newDeps = currentDeps.filter((depId) => depId !== sourceId); + + onUpdateFeature?.(targetId, { + dependencies: newDeps, + }); + + toast.success('Dependency removed'); + }, }), - [features, onViewOutput, onEditFeature, onStartTask, onStopTask, onResumeTask, onSpawnTask] + [ + features, + onViewOutput, + onEditFeature, + onStartTask, + onStopTask, + onResumeTask, + onSpawnTask, + onDeleteTask, + onUpdateFeature, + ] ); return ( diff --git a/apps/ui/src/components/views/graph-view/hooks/use-graph-layout.ts b/apps/ui/src/components/views/graph-view/hooks/use-graph-layout.ts index b44b7bcf..667ef737 100644 --- a/apps/ui/src/components/views/graph-view/hooks/use-graph-layout.ts +++ b/apps/ui/src/components/views/graph-view/hooks/use-graph-layout.ts @@ -1,4 +1,4 @@ -import { useCallback, useMemo } from 'react'; +import { useCallback, useMemo, useRef } from 'react'; import dagre from 'dagre'; import { Node, Edge, useReactFlow } from '@xyflow/react'; import { TaskNode, DependencyEdge } from './use-graph-nodes'; @@ -18,6 +18,10 @@ interface UseGraphLayoutProps { export function useGraphLayout({ nodes, edges }: UseGraphLayoutProps) { const { fitView, setNodes } = useReactFlow(); + // Cache the last computed positions to avoid recalculating layout + const positionCache = useRef>(new Map()); + const lastStructureKey = useRef(''); + const getLayoutedElements = useCallback( ( inputNodes: TaskNode[], @@ -48,12 +52,15 @@ export function useGraphLayout({ nodes, edges }: UseGraphLayoutProps) { const layoutedNodes = inputNodes.map((node) => { const nodeWithPosition = dagreGraph.node(node.id); + const position = { + x: nodeWithPosition.x - NODE_WIDTH / 2, + y: nodeWithPosition.y - NODE_HEIGHT / 2, + }; + // Update cache + positionCache.current.set(node.id, position); return { ...node, - position: { - x: nodeWithPosition.x - NODE_WIDTH / 2, - y: nodeWithPosition.y - NODE_HEIGHT / 2, - }, + position, targetPosition: isHorizontal ? 'left' : 'top', sourcePosition: isHorizontal ? 'right' : 'bottom', } as TaskNode; @@ -64,13 +71,45 @@ export function useGraphLayout({ nodes, edges }: UseGraphLayoutProps) { [] ); - // Initial layout + // Create a stable structure key based only on node IDs (not edge changes) + // Edges changing shouldn't trigger re-layout + const structureKey = useMemo(() => { + const nodeIds = nodes + .map((n) => n.id) + .sort() + .join(','); + return nodeIds; + }, [nodes]); + + // Initial layout - only recalculate when node structure changes (new nodes added/removed) const layoutedElements = useMemo(() => { if (nodes.length === 0) { + positionCache.current.clear(); + lastStructureKey.current = ''; return { nodes: [], edges: [] }; } - return getLayoutedElements(nodes, edges, 'LR'); - }, [nodes, edges, getLayoutedElements]); + + // Check if structure changed (new nodes added or removed) + const structureChanged = structureKey !== lastStructureKey.current; + + if (structureChanged) { + // Structure changed - run full layout + lastStructureKey.current = structureKey; + return getLayoutedElements(nodes, edges, 'LR'); + } else { + // Structure unchanged - preserve cached positions, just update node data + const layoutedNodes = nodes.map((node) => { + const cachedPosition = positionCache.current.get(node.id); + return { + ...node, + position: cachedPosition || { x: 0, y: 0 }, + targetPosition: 'left', + sourcePosition: 'right', + } as TaskNode; + }); + return { nodes: layoutedNodes, edges }; + } + }, [nodes, edges, structureKey, getLayoutedElements]); // Manual re-layout function const runLayout = useCallback( diff --git a/apps/ui/src/components/views/graph-view/hooks/use-graph-nodes.ts b/apps/ui/src/components/views/graph-view/hooks/use-graph-nodes.ts index e216462d..d9b340a9 100644 --- a/apps/ui/src/components/views/graph-view/hooks/use-graph-nodes.ts +++ b/apps/ui/src/components/views/graph-view/hooks/use-graph-nodes.ts @@ -25,6 +25,7 @@ export interface TaskNodeData extends Feature { onStopTask?: () => void; onResumeTask?: () => void; onSpawnTask?: () => void; + onDeleteTask?: () => void; } export type TaskNode = Node; @@ -33,6 +34,7 @@ export type DependencyEdge = Edge<{ targetStatus: Feature['status']; isHighlighted?: boolean; isDimmed?: boolean; + onDeleteDependency?: (sourceId: string, targetId: string) => void; }>; export interface NodeActionCallbacks { @@ -42,6 +44,8 @@ export interface NodeActionCallbacks { onStopTask?: (featureId: string) => void; onResumeTask?: (featureId: string) => void; onSpawnTask?: (featureId: string) => void; + onDeleteTask?: (featureId: string) => void; + onDeleteDependency?: (sourceId: string, targetId: string) => void; } interface UseGraphNodesProps { @@ -117,6 +121,9 @@ export function useGraphNodes({ onSpawnTask: actionCallbacks?.onSpawnTask ? () => actionCallbacks.onSpawnTask!(feature.id) : undefined, + onDeleteTask: actionCallbacks?.onDeleteTask + ? () => actionCallbacks.onDeleteTask!(feature.id) + : undefined, }, }; @@ -146,6 +153,7 @@ export function useGraphNodes({ targetStatus: feature.status, isHighlighted: edgeIsHighlighted, isDimmed: edgeIsDimmed, + onDeleteDependency: actionCallbacks?.onDeleteDependency, }, }; edgeList.push(edge); diff --git a/libs/dependency-resolver/src/resolver.ts b/libs/dependency-resolver/src/resolver.ts index c63b1a1a..f54524c0 100644 --- a/libs/dependency-resolver/src/resolver.ts +++ b/libs/dependency-resolver/src/resolver.ts @@ -212,7 +212,8 @@ export function getBlockingDependencies(feature: Feature, allFeatures: Feature[] /** * Checks if adding a dependency from sourceId to targetId would create a circular dependency. - * Uses DFS to detect if targetId can reach sourceId through existing dependencies. + * When we say "targetId depends on sourceId", we add sourceId to targetId.dependencies. + * A cycle would occur if sourceId already depends on targetId (directly or transitively). * * @param features - All features in the system * @param sourceId - The feature that would become a dependency (the prerequisite) @@ -227,22 +228,24 @@ export function wouldCreateCircularDependency( const featureMap = new Map(features.map((f) => [f.id, f])); const visited = new Set(); - function canReach(currentId: string, target: string): boolean { - if (currentId === target) return true; - if (visited.has(currentId)) return false; + // Check if 'from' can reach 'to' by following dependencies + function canReach(fromId: string, toId: string): boolean { + if (fromId === toId) return true; + if (visited.has(fromId)) return false; - visited.add(currentId); - const feature = featureMap.get(currentId); + visited.add(fromId); + const feature = featureMap.get(fromId); if (!feature?.dependencies) return false; for (const depId of feature.dependencies) { - if (canReach(depId, target)) return true; + if (canReach(depId, toId)) return true; } return false; } - // Check if source can reach target through existing dependencies - // If so, adding target -> source would create a cycle + // We want to add: targetId depends on sourceId (sourceId -> targetId in dependency graph) + // This would create a cycle if sourceId already depends on targetId (transitively) + // i.e., if we can reach targetId starting from sourceId by following dependencies return canReach(sourceId, targetId); } @@ -321,8 +324,10 @@ export function getAncestors( /** * Formats ancestor context for inclusion in a task description. + * The parent task (depth=-1) is formatted with special emphasis indicating + * it was already completed and is provided for context only. * - * @param ancestors - Array of ancestor contexts (including parent) + * @param ancestors - Array of ancestor contexts (including parent with depth=-1) * @param selectedIds - Set of selected ancestor IDs to include * @returns Formatted markdown string with ancestor context */ @@ -333,24 +338,59 @@ export function formatAncestorContextForPrompt( const selectedAncestors = ancestors.filter((a) => selectedIds.has(a.id)); if (selectedAncestors.length === 0) return ''; - const sections = selectedAncestors.map((ancestor) => { - const parts: string[] = []; - const title = ancestor.title || `Task (${ancestor.id.slice(0, 8)})`; + // Separate parent (depth=-1) from other ancestors + const parent = selectedAncestors.find((a) => a.depth === -1); + const otherAncestors = selectedAncestors.filter((a) => a.depth !== -1); - parts.push(`### ${title}`); + const sections: string[] = []; - if (ancestor.description) { - parts.push(`**Description:** ${ancestor.description}`); + // Format parent with special emphasis + if (parent) { + const parentTitle = parent.title || `Task (${parent.id.slice(0, 8)})`; + const parentParts: string[] = []; + + parentParts.push(`## Parent Task Context (Already Completed)`); + parentParts.push( + `> **Note:** The following parent task has already been completed. This context is provided to help you understand the background and requirements for this sub-task. Do not re-implement the parent task - focus only on the new sub-task described below.` + ); + parentParts.push(`### ${parentTitle}`); + + if (parent.description) { + parentParts.push(`**Description:** ${parent.description}`); } - if (ancestor.spec) { - parts.push(`**Specification:**\n${ancestor.spec}`); + if (parent.spec) { + parentParts.push(`**Specification:**\n${parent.spec}`); } - if (ancestor.summary) { - parts.push(`**Summary:** ${ancestor.summary}`); + if (parent.summary) { + parentParts.push(`**Summary:** ${parent.summary}`); } - return parts.join('\n\n'); - }); + sections.push(parentParts.join('\n\n')); + } - return `## Ancestor Context\n\n${sections.join('\n\n---\n\n')}`; + // Format other ancestors if any + if (otherAncestors.length > 0) { + const ancestorSections = otherAncestors.map((ancestor) => { + const parts: string[] = []; + const title = ancestor.title || `Task (${ancestor.id.slice(0, 8)})`; + + parts.push(`### ${title}`); + + if (ancestor.description) { + parts.push(`**Description:** ${ancestor.description}`); + } + if (ancestor.spec) { + parts.push(`**Specification:**\n${ancestor.spec}`); + } + if (ancestor.summary) { + parts.push(`**Summary:** ${ancestor.summary}`); + } + + return parts.join('\n\n'); + }); + + sections.push(`## Additional Ancestor Context\n\n${ancestorSections.join('\n\n---\n\n')}`); + } + + return sections.join('\n\n---\n\n'); } diff --git a/libs/dependency-resolver/tests/resolver.test.ts b/libs/dependency-resolver/tests/resolver.test.ts index a44669e5..5f246b2a 100644 --- a/libs/dependency-resolver/tests/resolver.test.ts +++ b/libs/dependency-resolver/tests/resolver.test.ts @@ -3,6 +3,8 @@ import { resolveDependencies, areDependenciesSatisfied, getBlockingDependencies, + wouldCreateCircularDependency, + dependencyExists, } from '../src/resolver'; import type { Feature } from '@automaker/types'; @@ -348,4 +350,204 @@ describe('resolver.ts', () => { expect(blocking).not.toContain('Dep2'); }); }); + + describe('wouldCreateCircularDependency', () => { + it('should return false for features with no existing dependencies', () => { + const features = [createFeature('A'), createFeature('B')]; + + // Making B depend on A should not create a cycle + expect(wouldCreateCircularDependency(features, 'A', 'B')).toBe(false); + }); + + it('should return false for valid linear dependency chain', () => { + // A <- B <- C (C depends on B, B depends on A) + const features = [ + createFeature('A'), + createFeature('B', { dependencies: ['A'] }), + createFeature('C', { dependencies: ['B'] }), + ]; + + // Making D depend on C should not create a cycle + const featuresWithD = [...features, createFeature('D')]; + expect(wouldCreateCircularDependency(featuresWithD, 'C', 'D')).toBe(false); + }); + + it('should detect direct circular dependency (A -> B -> A)', () => { + // B depends on A + const features = [createFeature('A'), createFeature('B', { dependencies: ['A'] })]; + + // Making A depend on B would create: A -> B -> A (cycle!) + // sourceId = B (prerequisite), targetId = A (will depend on B) + // This creates a cycle because B already depends on A + expect(wouldCreateCircularDependency(features, 'B', 'A')).toBe(true); + }); + + it('should detect transitive circular dependency (A -> B -> C -> A)', () => { + // C depends on B, B depends on A + const features = [ + createFeature('A'), + createFeature('B', { dependencies: ['A'] }), + createFeature('C', { dependencies: ['B'] }), + ]; + + // Making A depend on C would create: A -> C -> B -> A (cycle!) + // sourceId = C (prerequisite), targetId = A (will depend on C) + expect(wouldCreateCircularDependency(features, 'C', 'A')).toBe(true); + }); + + it('should detect cycle in complex graph', () => { + // Graph: A <- B, A <- C, B <- C (C depends on both A and B, B depends on A) + const features = [ + createFeature('A'), + createFeature('B', { dependencies: ['A'] }), + createFeature('C', { dependencies: ['A', 'B'] }), + ]; + + // Making A depend on C would create a cycle + expect(wouldCreateCircularDependency(features, 'C', 'A')).toBe(true); + + // Making B depend on C would also create a cycle + expect(wouldCreateCircularDependency(features, 'C', 'B')).toBe(true); + }); + + it('should return false for parallel branches', () => { + // A <- B, A <- C (B and C both depend on A, but not on each other) + const features = [ + createFeature('A'), + createFeature('B', { dependencies: ['A'] }), + createFeature('C', { dependencies: ['A'] }), + ]; + + // Making B depend on C should be fine (no cycle) + expect(wouldCreateCircularDependency(features, 'C', 'B')).toBe(false); + + // Making C depend on B should also be fine + expect(wouldCreateCircularDependency(features, 'B', 'C')).toBe(false); + }); + + it('should handle self-dependency check', () => { + const features = [createFeature('A')]; + + // A depending on itself would be a trivial cycle + expect(wouldCreateCircularDependency(features, 'A', 'A')).toBe(true); + }); + + it('should handle feature not in list', () => { + const features = [createFeature('A')]; + + // Non-existent source - should return false (no path exists) + expect(wouldCreateCircularDependency(features, 'NonExistent', 'A')).toBe(false); + + // Non-existent target - should return false + expect(wouldCreateCircularDependency(features, 'A', 'NonExistent')).toBe(false); + }); + + it('should handle empty features list', () => { + const features: Feature[] = []; + + expect(wouldCreateCircularDependency(features, 'A', 'B')).toBe(false); + }); + + it('should handle longer transitive chains', () => { + // A <- B <- C <- D <- E + const features = [ + createFeature('A'), + createFeature('B', { dependencies: ['A'] }), + createFeature('C', { dependencies: ['B'] }), + createFeature('D', { dependencies: ['C'] }), + createFeature('E', { dependencies: ['D'] }), + ]; + + // Making A depend on E would create a 5-node cycle + expect(wouldCreateCircularDependency(features, 'E', 'A')).toBe(true); + + // Making B depend on E would create a 4-node cycle + expect(wouldCreateCircularDependency(features, 'E', 'B')).toBe(true); + + // Making E depend on A is fine (already exists transitively, but adding explicit is ok) + // Wait, E already depends on A transitively. Let's add F instead + const featuresWithF = [...features, createFeature('F')]; + expect(wouldCreateCircularDependency(featuresWithF, 'E', 'F')).toBe(false); + }); + + it('should handle diamond dependency pattern', () => { + // A + // / \ + // B C + // \ / + // D + const features = [ + createFeature('A'), + createFeature('B', { dependencies: ['A'] }), + createFeature('C', { dependencies: ['A'] }), + createFeature('D', { dependencies: ['B', 'C'] }), + ]; + + // Making A depend on D would create a cycle through both paths + expect(wouldCreateCircularDependency(features, 'D', 'A')).toBe(true); + + // Making B depend on D would create a cycle + expect(wouldCreateCircularDependency(features, 'D', 'B')).toBe(true); + + // Adding E that depends on D should be fine + const featuresWithE = [...features, createFeature('E')]; + expect(wouldCreateCircularDependency(featuresWithE, 'D', 'E')).toBe(false); + }); + }); + + describe('dependencyExists', () => { + it('should return false when target has no dependencies', () => { + const features = [createFeature('A'), createFeature('B')]; + + expect(dependencyExists(features, 'A', 'B')).toBe(false); + }); + + it('should return true when direct dependency exists', () => { + const features = [createFeature('A'), createFeature('B', { dependencies: ['A'] })]; + + expect(dependencyExists(features, 'A', 'B')).toBe(true); + }); + + it('should return false for reverse direction', () => { + const features = [createFeature('A'), createFeature('B', { dependencies: ['A'] })]; + + // B depends on A, but A does not depend on B + expect(dependencyExists(features, 'B', 'A')).toBe(false); + }); + + it('should return false for transitive dependencies', () => { + // This function only checks direct dependencies, not transitive + const features = [ + createFeature('A'), + createFeature('B', { dependencies: ['A'] }), + createFeature('C', { dependencies: ['B'] }), + ]; + + // C depends on B which depends on A, but C doesn't directly depend on A + expect(dependencyExists(features, 'A', 'C')).toBe(false); + }); + + it('should return true for one of multiple dependencies', () => { + const features = [ + createFeature('A'), + createFeature('B'), + createFeature('C', { dependencies: ['A', 'B'] }), + ]; + + expect(dependencyExists(features, 'A', 'C')).toBe(true); + expect(dependencyExists(features, 'B', 'C')).toBe(true); + }); + + it('should return false when target feature does not exist', () => { + const features = [createFeature('A')]; + + expect(dependencyExists(features, 'A', 'NonExistent')).toBe(false); + }); + + it('should return false for empty dependencies array', () => { + const features = [createFeature('A'), createFeature('B', { dependencies: [] })]; + + expect(dependencyExists(features, 'A', 'B')).toBe(false); + }); + }); });