ADFA-4329: Guard module-graph traversal against dependency cycles (StackOverflow)

[fryanpan]

Jira Ticket: https://appdevforall.atlassian.net/browse/ADFA-4329
Sentry Issue: https://appdevforall-inc-9p.sentry.io/issues/APPDEVFORALL-Z0

Note

This stops the crash, but only logs the dependency cycle into developer logs without showing the user anything obvious. When the user actually tries to build the project, Gradle should surface a more user friendly and visible message into the Build Output Panel.

Was reviewing with Claude if there might be an easy way to make this error more visible at this point -- but it seemed like this is happening in a layer where there isn't easy access to show something in the UI.

Reproduction Details

ModuleProject.getCompileModuleProjects() (and the sibling getCompileClasspaths()) walk the project dependency graph and recurse with no module-level visited set. A cyclic project graph (:a → :b → :a, or a module that is its own transitive project dependency) recurses until the JVM stack overflows.

Stack Trace

StackOverflowError: stack size 1039KB
  at com.itsaky.androidide.projects.api.AndroidModule.getCompileModuleProjects(Unknown Source:107)
  at com.itsaky.androidide.projects.api.AndroidModule.getCompileModuleProjects(Unknown Source:107)
  ... (the getCompileModuleProjects frame repeats ~90× until the stack is exhausted) ...
  at com.itsaky.androidide.projects.api.AndroidModule.getCompileModuleProjects(Unknown Source:58)
  at java.util.LinkedHashMap.get(LinkedHashMap.java:554)

(transaction: EditorActivityKt · device.class: medium · Android 14)

User Steps

User steps leading up to crash, based on Sentry breadcrumbs:

• User leaves the editor for the project list, then re-opens a project (EditorActivityKt paused → MainActivity → EditorActivityKt created→started→resumed).
• On re-open the background indexer starts (BackgroundIndexer: Closing indexer with 2 active job(s)) and the project model is traversed; the opened project has a cyclic module dependency, so the recursive graph walk overflows the stack during load.

Was able to reproduce in a unit test?

Yes.
CompileModuleProjectsCycleTest (:subprojects:projects) builds a cycle of real AndroidModule instances over protobuf project models and drives the production traversal. On the pre-fix baseline all cycle cases fail with StackOverflowError; on the branch they terminate. Added getCompileClasspaths two-node + self-cycle termination tests too.

What Was Fixed

• Thread a visited: MutableSet<String> (module path) through getCompileModuleProjects in ModuleProject/AndroidModule/JavaModule; each module expands at most once. Public no-arg API delegates with a fresh set.
• Distinguish a true cycle (module already on the DFS recursion path) from a harmless diamond/shared dependency, and report the cycle as an actionable error (Module dependency cycle detected: :a -> :b -> :a …) instead of silently swallowing it (new UX, ModuleProject.reportDependencyCycle + pure formatDependencyCycle).
• Apply the same module-path guard to the sibling getCompileClasspaths traversal (AndroidModule.collectLibraries cross-module call + JavaModule), which had the identical SOE on classpath resolution.

Testing

:subprojects:projects:testV8DebugUnitTest --tests CompileModuleProjectsCycleTest → 7/7 green (3 module-cycle termination, diamond-not-a-cycle, cycle-chain formatter, 2 classpath-cycle termination). Verified red→green by reverting only the prod files to baseline. Local CodeRabbit review: no findings.

---

Fixes APPDEVFORALL-Z0

[coderabbitai]

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info

⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: 8b1bc877-5842-4da3-ab4e-cb69b6bd9ec1

📥 Commits

Reviewing files that changed from the base of the PR and between d73ec0b and b9b4279.

📒 Files selected for processing (4)

• subprojects/projects/src/main/java/com/itsaky/androidide/projects/api/AndroidModule.kt
• subprojects/projects/src/main/java/com/itsaky/androidide/projects/api/JavaModule.kt
• subprojects/projects/src/main/java/com/itsaky/androidide/projects/api/ModuleProject.kt
• subprojects/projects/src/test/java/com/itsaky/androidide/projects/api/CompileModuleProjectsCycleTest.kt

---

📝 Walkthrough

Release Notes

Bug Fixes

• Critical: Fixed StackOverflowError (APPDEVFORALL-Z0) that occurred when the IDE attempted to traverse project dependency graphs containing cycles (e.g., :a → :b → :a or self-referential cycles)
  • Root cause: ModuleProject.getCompileModuleProjects() and getCompileClasspaths() recursively walked dependency graphs without tracking visited modules
  • Solution: Thread a visited parameter through traversal methods to ensure each module is expanded at most once

Improvements

• Enhanced cycle detection to distinguish between true cycles and harmless diamond dependencies (shared transitive dependencies)
• Added actionable error reporting for detected dependency cycles with formatted cycle chains (e.g., :a -> :b -> :a) logged to developer logs
• Gradle will provide a user-friendly message in the Build Output Panel during project builds

Testing

• Added comprehensive unit test suite (CompileModuleProjectsCycleTest) with 7 test cases using real AndroidModule instances:
  • Verify two-node and longer cycles terminate without StackOverflow
  • Verify self-referential cycles are properly detected
  • Verify diamond dependencies are not misreported as cycles
  • Verify getCompileClasspaths properly terminates on cyclic module graphs
  • Verify cycle formatter correctly renders dependency chains

⚠️ Implementer Breaking Changes

• Abstract method signature changes in ModuleProject:
  • abstract fun getCompileModuleProjects(): List<ModuleProject> → abstract fun getCompileModuleProjects(visited: MutableSet<String>, recursionPath: ArrayDeque<String>): List<ModuleProject>
  • abstract fun getCompileClasspaths(excludeSourceGeneratedClassPath: Boolean): Set<File> → abstract fun getCompileClasspaths(excludeSourceGeneratedClassPath: Boolean, visited: MutableSet<String>): Set<File>
  • Risk: Any custom subclasses of ModuleProject must update their implementations to match new abstract method signatures
  • Violation of best practices: Adding required parameters to abstract methods is a breaking change affecting all subclasses and third-party implementations

Backward Compatibility

• Public caller API remains fully backward compatible:
  • Concrete wrapper methods getCompileModuleProjects() and getCompileClasspaths(excludeSourceGeneratedClassPath) remain unchanged and delegate to abstract methods with initialized parameters
  • Existing client code calling these public methods continues to work without modification
• All internal implementations (AndroidModule, JavaModule) have been updated to match new signatures

Walkthrough

Compile classpath and module project traversal in ModuleProject, AndroidModule, and JavaModule are updated to accept visited and recursionPath parameters, enabling DFS cycle detection and diamond-deduplication. Public no-arg wrappers in ModuleProject seed these parameters. A new Robolectric test suite validates termination across all cycle shapes.

Changes

Compile Dependency Cycle Detection

Layer / File(s)	Summary
ModuleProject: cycle helpers and updated abstract API subprojects/projects/src/main/java/com/itsaky/androidide/projects/api/ModuleProject.kt	Adds formatDependencyCycle and reportDependencyCycle helpers; changes abstract getCompileClasspaths and getCompileModuleProjects signatures to require visited/recursionPath; adds concrete no-arg wrappers that seed those parameters before delegating to the abstract implementations.
AndroidModule: cycle-safe classpath and module traversal subprojects/projects/src/main/java/com/itsaky/androidide/projects/api/AndroidModule.kt	getCompileClasspaths checks visited and returns empty on re-entry; collectLibraries threads per-module visited and cross-module moduleVisited sets, recursing into getCompileClasspaths(..., moduleVisited) for PROJECT dependencies; getCompileModuleProjects uses recursionPath.contains for true-cycle detection and visited.add for diamond deduplication with a try/finally removeLast pattern.
JavaModule: cycle-safe classpath and module traversal subprojects/projects/src/main/java/com/itsaky/androidide/projects/api/JavaModule.kt	getCompileClasspaths guards against revisiting module paths and propagates visited into compile-scope module recursive calls; getCompileModuleProjects adds recursionPath cycle detection and visited deduplication matching the AndroidModule pattern.
Cycle and diamond termination tests subprojects/projects/src/test/java/com/itsaky/androidide/projects/api/CompileModuleProjectsCycleTest.kt	New Robolectric test suite with helpers to construct AndroidModule instances and install a ProjectManagerImpl workspace; covers two-node cycles, self-dependency, longer cycles, diamond shared dependencies, getCompileClasspaths termination, and formatDependencyCycle output rendering.

Sequence Diagram(s)

sequenceDiagram
  participant Caller
  participant ModuleProject
  participant AndroidModule

  Caller->>ModuleProject: getCompileModuleProjects()
  ModuleProject->>ModuleProject: visited = mutableSetOf()<br/>recursionPath = ArrayDeque()
  ModuleProject->>AndroidModule: getCompileModuleProjects(visited, recursionPath)

  rect rgba(255, 100, 100, 0.5)
    note over AndroidModule: Cycle check
    AndroidModule->>AndroidModule: recursionPath.contains(path)?
    AndroidModule-->>ModuleProject: reportDependencyCycle() → return emptyList()
  end

  rect rgba(100, 200, 100, 0.5)
    note over AndroidModule: Diamond dedup
    AndroidModule->>AndroidModule: visited.add(path) returns false → skip
  end

  AndroidModule->>AndroidModule: recursionPath.addLast(path)
  AndroidModule->>AndroidModule: recurse into dependencies
  AndroidModule->>AndroidModule: recursionPath.removeLast() [finally]
  AndroidModule-->>Caller: List<ModuleProject>

Loading

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~25 minutes

Suggested reviewers

• Daniel-ADFA

Poem

🐇 Hop, hop — no infinite loops today!
The visited set keeps cycles at bay,
:a -> :b -> :a? We see through your trick!
recursionPath catches it, slick as a click.
Diamond deps pass, no false alarm cried —
The bunny lands safe on the other side! 🌿

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name	Status	Explanation	Resolution
Docstring Coverage	⚠️ Warning	Docstring coverage is 59.26% which is insufficient. The required threshold is 80.00%.	Write docstrings for the functions missing them to satisfy the coverage threshold.

✅ Passed checks (4 passed)

Check name	Status	Explanation
Title check	✅ Passed	The title clearly and specifically describes the main change: guarding module-graph traversal against dependency cycles to prevent StackOverflow errors.
Description check	✅ Passed	The description comprehensively explains the issue, fix, testing approach, and user impact in relation to the changeset modifications.
Linked Issues check	✅ Passed	Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check	✅ Passed	Check skipped because no linked issues were found for this pull request.

_{✏️ Tip: You can configure your own custom pre-merge checks in the settings.}

✨ Finishing Touches

📝 Generate docstrings

• Create stacked PR
• Commit on current branch

🧪 Generate unit tests (beta)

• Create PR with unit tests
• Commit unit tests in branch ADFA-4329-getcompilemoduleprojects-stackoverflow

---

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

_{Comment @coderabbitai help to get the list of available commands and usage tips.}

[itsaky-adfa]

The previous behavior was an oversight, so the excludeSourceGeneratedClassPath should be propagated here. Could you please update this so the flag is propagated?

[itsaky-adfa]

The newly added functions accept visited and recursionPath parameters, which seem to be something that the callers don't need to know about. They also seem to be only called internally. Can we make the newly added overloads protected?