fix(builder): infinite loop possible on cycle in chain definition

[dbeal-eth]

due to the way that the call patterns work, it was very possible to have a cycle if dependencies were defined incorrectly.

fixes https://linear.app/usecannon/issue/CAN-763/builder-fails-to-detect-cycle-in-cannonfile

[codecov]

Codecov Report

✅ All modified and coverable lines are covered by tests.

📢 Thoughts on this report? Let us know!

[dbeal-eth]

@claude please review

[claude]

Claude finished @dbeal-eth's task —— View job

---

Code Review: fix(builder): infinite loop possible on cycle in chain definition ✅

Review Checklist:

• Read repository guidelines (CLAUDE.md)
• Analyze PR diff and understand the changes
• Review implementation for correctness and best practices
• Examine test coverage and quality
• Provide comprehensive feedback

---

Summary

This PR successfully fixes an infinite loop issue in the builder by adding cycle detection to the dependency computation process. The implementation is sound and well-tested.

✅ Strengths

1. Correct Fix Placement: The cycle check is properly placed in initializeComputedDependencies() at line 346-349, right after dependency computation but before root/leaf calculations. This ensures cycles are caught early in the process.

2. Excellent Error Message: The error message is clear and actionable: "cannot generate dependency tree: dependency cycle found: ${cycles.join(' => ')}". It shows the exact cycle path to help developers debug.

3. Robust Test Coverage: The existing checkCycles() method already has comprehensive tests covering:

   • Empty graphs (0 nodes)
   • Diamond dependency patterns (no cycles)
   • Self-referencing cycles (1 node)
   • Two-node cycles
   • Complex three-node cycles with other valid dependencies

4. Safe Test Helper Update: The modification to makeFakeChainDefinition() is well-designed:

   try {
     def.initializeComputedDependencies();
   } catch {
     // no contents
   }

   This allows cycle tests to work properly by catching the new error while maintaining the existing API.

🔍 Technical Analysis

Cycle Detection Algorithm (definition.ts:484-518):

• Uses depth-first search with path tracking
• currentPath detects back edges (cycles)
• seenNodes avoids redundant traversals
• Returns the first cycle found with proper path reconstruction
• Time complexity: O(V + E) where V is nodes and E is edges

Integration Point (definition.ts:346-349):

• Properly positioned after resolvedDependencies is populated
• Throws immediately on cycle detection, preventing infinite loops
• Maintains existing error handling patterns in the codebase

📝 Minor Observations

1. Empty Catch Block: The test helper uses an empty catch block, but this is acceptable since it's specifically for testing cycle detection and the comment indicates this is intentional.

2. Consistent Error Format: The error message format matches other dependency-related errors in the same method (like output clashes).

🎯 Impact Assessment

• Fixes Critical Issue: Resolves infinite loops that could hang the builder process
• Backward Compatible: No breaking changes to existing APIs
• Performance: Minimal overhead - cycle detection only runs during initialization
• User Experience: Clear error messages help developers identify and fix dependency issues

Verdict: APPROVED ✅

This is a well-implemented fix that addresses the reported issue (CAN-763) effectively. The code is correct, well-tested, and follows the project's conventions. The cycle detection prevents infinite loops while providing helpful error messages for debugging.

---

Review completed for branch: fix/def-infinite-cycle