From 5568b7f27b9ba32b0debc8e9ddc8206a93c96ec1 Mon Sep 17 00:00:00 2001
From: Todd Burnside <tburnside@gemini.edu>
Date: Mon, 3 Aug 2026 10:49:29 -0700
Subject: [PATCH] Don't report a shared fragment as a fragment cycle

`validateVariablesAndFragments` accumulated visited fragments into a single
`seen` set spanning the whole traversal rather than tracking the fragments on
the current path. A fragment reachable by more than one path was therefore
reported as a cycle, e.g.

    fragment a on T { x { ...c } y { ...b } }
    fragment b on U { z { ...c } }
    fragment c on V { w }

failed with "Fragment cycle starting from 'a'".

Replace the check with a proper DFS: `path` holds the fragments on the current
traversal path and `done` those already explored in full, a fragment joins
`done` only after its references have been explored, and the `path` check
precedes the `done` check. Genuine recursion is still detected, and reported
against the same fragment as before.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01Lyt6Er1f7k4DDcNBoZEUGS
---
 modules/core/src/main/scala/compiler.scala    | 52 ++++++++--------
 .../test/scala/compiler/FragmentSuite.scala   | 61 +++++++++++++++++++
 2 files changed, 88 insertions(+), 25 deletions(-)

diff --git a/modules/core/src/main/scala/compiler.scala b/modules/core/src/main/scala/compiler.scala
index 3bdf6d3d..9ee69868 100644
--- a/modules/core/src/main/scala/compiler.scala
+++ b/modules/core/src/main/scala/compiler.scala
@@ -445,34 +445,36 @@ class QueryCompiler(parser: QueryParser, schema: Schema, phases: List[Phase]) {
       val fragRefs: Map[String, (Set[String], Set[String])] =
         frags.map { frag => (frag.name, collectQueryRefs(frag.child)) }.toMap
 
-      @tailrec
-      def checkCycle(pendingFrags: Set[String], seen: Set[String]): Option[Set[String]] = {
-        if (pendingFrags.isEmpty) Some(seen)
-        else {
-          val hd = pendingFrags.head
-          if (seen.contains(hd)) None
+      // Find a fragment which is reachable from itself, if any.
+      //
+      // `path` is the set of fragments on the current traversal path, and `done` the set of
+      // fragments whose references have already been explored in full without discovering a
+      // cycle. A fragment is added to `done` only once its references have been explored, and
+      // the `path` check precedes the `done` check, so that a fragment which is merely
+      // reachable by more than one path isn't mistaken for a cycle.
+      def findCycle: Option[String] = {
+        def visit(
+            frag: String,
+            path: Set[String],
+            done: Set[String]): Either[String, Set[String]] =
+          if (path.contains(frag)) Left(frag)
+          else if (done.contains(frag)) Right(done)
           else
-            checkCycle(
-              fragRefs.get(hd).map(_._2).getOrElse(Set.empty) ++ pendingFrags.tail,
-              seen + hd)
-        }
-      }
+            fragRefs
+              .get(frag)
+              .map(_._2)
+              .getOrElse(Set.empty)
+              .foldLeft(Right(done): Either[String, Set[String]]) { (acc, ref) =>
+                acc.flatMap(visit(ref, path + frag, _))
+              }
+              .map(_ + frag)
 
-      def findCycle: Option[String] = {
-        @tailrec
-        def loop(pendingFrags: Set[String]): Either[Set[String], String] = {
-          if (pendingFrags.isEmpty) Left(Set.empty[String])
-          else {
-            val hd = pendingFrags.head
-            checkCycle(Set(hd), Set.empty[String]) match {
-              case None => Right(hd)
-              case Some(seen) => loop(pendingFrags.tail.diff(seen))
-            }
+        uniqueFrags
+          .foldLeft(Right(Set.empty[String]): Either[String, Set[String]]) { (acc, frag) =>
+            acc.flatMap(visit(frag, Set.empty[String], _))
           }
-        }
-
-        if (uniqueFrags.isEmpty) None
-        else loop(uniqueFrags).toOption
+          .left
+          .toOption
       }
 
       findCycle match {
diff --git a/modules/core/src/test/scala/compiler/FragmentSuite.scala b/modules/core/src/test/scala/compiler/FragmentSuite.scala
index fa52476b..1e139ab5 100644
--- a/modules/core/src/test/scala/compiler/FragmentSuite.scala
+++ b/modules/core/src/test/scala/compiler/FragmentSuite.scala
@@ -1152,6 +1152,67 @@ final class FragmentSuite extends CatsEffectSuite {
     assertIO(res, expected)
   }
 
+  test("fragment shared by multiple paths (not a cycle)") {
+    val query = """
+      query withFragments {
+        user(id: 1) {
+          ...userFields
+        }
+      }
+
+      fragment userFields on User {
+        friends {
+          ...nameFields
+        }
+        mutualFriends {
+          ...mutualFriendFields
+        }
+      }
+
+      fragment mutualFriendFields on User {
+        ...nameFields
+      }
+
+      fragment nameFields on User {
+        id
+        name
+      }
+    """
+
+    val expected = json"""
+      {
+        "data" : {
+          "user" : {
+            "friends" : [
+              {
+                "id" : "2",
+                "name" : "Bob"
+              },
+              {
+                "id" : "3",
+                "name" : "Carol"
+              }
+            ],
+            "mutualFriends" : [
+              {
+                "id" : "2",
+                "name" : "Bob"
+              },
+              {
+                "id" : "3",
+                "name" : "Carol"
+              }
+            ]
+          }
+        }
+      }
+    """
+
+    val res = FragmentMapping.compileAndRun(query)
+
+    assertIO(res, expected)
+  }
+
   test("fragment recursion (1)") {
     val query = """
       query withFragments {