Merge pull request #243 from JBotwina/JBotwina/task-deps-spawn

feat: Add task dependencies and spawn sub-task functionality

libs/dependency-resolver/src/index.ts

@@ -7,5 +7,10 @@ export {
   resolveDependencies,
   areDependenciesSatisfied,
   getBlockingDependencies,
+  wouldCreateCircularDependency,
+  dependencyExists,
+  getAncestors,
+  formatAncestorContextForPrompt,
   type DependencyResolutionResult,
+  type AncestorContext,
 } from './resolver.js';

libs/dependency-resolver/src/resolver.ts

@@ -209,3 +209,188 @@ export function getBlockingDependencies(feature: Feature, allFeatures: Feature[]
     return dep && dep.status !== 'completed' && dep.status !== 'verified';
   });
 }
+
+/**
+ * Checks if adding a dependency from sourceId to targetId would create a circular dependency.
+ * 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)
+ * @param targetId - The feature that would depend on sourceId
+ * @returns true if adding this dependency would create a cycle
+ */
+export function wouldCreateCircularDependency(
+  features: Feature[],
+  sourceId: string,
+  targetId: string
+): boolean {
+  const featureMap = new Map(features.map((f) => [f.id, f]));
+  const visited = new Set<string>();
+
+  // 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(fromId);
+    const feature = featureMap.get(fromId);
+    if (!feature?.dependencies) return false;
+
+    for (const depId of feature.dependencies) {
+      if (canReach(depId, toId)) return true;
+    }
+    return false;
+  }
+
+  // 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);
+}
+
+/**
+ * Checks if a dependency already exists between two features.
+ *
+ * @param features - All features in the system
+ * @param sourceId - The potential dependency (prerequisite)
+ * @param targetId - The feature that might depend on sourceId
+ * @returns true if targetId already depends on sourceId
+ */
+export function dependencyExists(features: Feature[], sourceId: string, targetId: string): boolean {
+  const targetFeature = features.find((f) => f.id === targetId);
+  if (!targetFeature?.dependencies) return false;
+  return targetFeature.dependencies.includes(sourceId);
+}
+
+/**
+ * Context information about an ancestor feature in the dependency graph.
+ */
+export interface AncestorContext {
+  id: string;
+  title?: string;
+  description: string;
+  spec?: string;
+  summary?: string;
+  depth: number; // 0 = immediate parent, 1 = grandparent, etc.
+}
+
+/**
+ * Traverses the dependency graph to find all ancestors of a feature.
+ * Returns ancestors ordered by depth (closest first).
+ *
+ * @param feature - The feature to find ancestors for
+ * @param allFeatures - All features in the system
+ * @param maxDepth - Maximum depth to traverse (prevents infinite loops)
+ * @returns Array of ancestor contexts, sorted by depth (closest first)
+ */
+export function getAncestors(
+  feature: Feature,
+  allFeatures: Feature[],
+  maxDepth: number = 10
+): AncestorContext[] {
+  const featureMap = new Map(allFeatures.map((f) => [f.id, f]));
+  const ancestors: AncestorContext[] = [];
+  const visited = new Set<string>();
+
+  function traverse(featureId: string, depth: number) {
+    if (depth > maxDepth || visited.has(featureId)) return;
+    visited.add(featureId);
+
+    const f = featureMap.get(featureId);
+    if (!f?.dependencies) return;
+
+    for (const depId of f.dependencies) {
+      const dep = featureMap.get(depId);
+      if (dep && !visited.has(depId)) {
+        ancestors.push({
+          id: dep.id,
+          title: dep.title,
+          description: dep.description,
+          spec: dep.spec,
+          summary: dep.summary,
+          depth,
+        });
+        traverse(depId, depth + 1);
+      }
+    }
+  }
+
+  traverse(feature.id, 0);
+
+  // Sort by depth (closest ancestors first)
+  return ancestors.sort((a, b) => a.depth - b.depth);
+}
+
+/**
+ * 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 with depth=-1)
+ * @param selectedIds - Set of selected ancestor IDs to include
+ * @returns Formatted markdown string with ancestor context
+ */
+export function formatAncestorContextForPrompt(
+  ancestors: AncestorContext[],
+  selectedIds: Set<string>
+): string {
+  const selectedAncestors = ancestors.filter((a) => selectedIds.has(a.id));
+  if (selectedAncestors.length === 0) return '';
+
+  // Separate parent (depth=-1) from other ancestors
+  const parent = selectedAncestors.find((a) => a.depth === -1);
+  const otherAncestors = selectedAncestors.filter((a) => a.depth !== -1);
+
+  const sections: string[] = [];
+
+  // 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 (parent.spec) {
+      parentParts.push(`**Specification:**\n${parent.spec}`);
+    }
+    if (parent.summary) {
+      parentParts.push(`**Summary:** ${parent.summary}`);
+    }
+
+    sections.push(parentParts.join('\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');
+}

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);
+    });
+  });
 });