From badbbea538afe12d6b873bda8304af0922b53493 Mon Sep 17 00:00:00 2001
From: Dobes Vandermeer <dobes@formative.com>
Date: Mon, 31 Mar 2025 16:28:25 -0700
Subject: [PATCH 1/2] Fix issue with noop tasks that depend on other noop tasks

If you had a runnable task that depended on a noop task that in tern depended on a noop task, that depended on a runnable task, the dependency between the two runnable tasks would not be preserved.  The logic that prunes out noop tasks in the dependency tree did not account for multiple levels of removal.
---
 packages/e2e-tests/src/basic.test.ts     | 32 ++++++++++++++++++++++++
 packages/target-graph/src/removeNodes.ts | 28 ++++++++++++++++-----
 2 files changed, 54 insertions(+), 6 deletions(-)

diff --git a/packages/e2e-tests/src/basic.test.ts b/packages/e2e-tests/src/basic.test.ts
index ad6195ae1..f66069ea8 100644
--- a/packages/e2e-tests/src/basic.test.ts
+++ b/packages/e2e-tests/src/basic.test.ts
@@ -1,5 +1,6 @@
 import { Monorepo } from "./mock/monorepo.js";
 import { filterEntry, parseNdJson } from "./parseNdJson.js";
+import { getTargetId } from "@lage-run/target-graph";
 
 describe("basics", () => {
   let repo: Monorepo | undefined;
@@ -130,4 +131,35 @@ describe("basics", () => {
     expect(jsonOutput.find((entry) => filterEntry(entry.data, "a", "test", "success"))).toBeTruthy();
     expect(jsonOutput.find((entry) => filterEntry(entry.data, "a", "lint", "success"))).toBeFalsy();
   });
+
+  it("handles multiple levels of noop tasks", async () => {
+    const repo = new Monorepo("noop-task-layers");
+
+    repo.init();
+    repo.setLageConfig(`module.exports = {
+      "pipeline": {
+        "build": [],
+        "_build": { "type": "noop", "dependsOn": ["build"] },
+        "__build": { "type": "noop", "dependsOn": ["_build", "^__build"] },
+        "test": ["__build"]         
+      },
+      npmClient: 'yarn'
+    }`);
+
+    repo.addPackage("a", ["b"]);
+    repo.addPackage("b");
+    repo.install();
+
+    const results = repo.run("test");
+    const output = results.stdout + results.stderr;
+
+    const jsonOutput = parseNdJson(output);
+
+    expect(jsonOutput.find((entry) => filterEntry(entry.data, "b", "build", "success"))).toBeTruthy();
+    expect(jsonOutput.find((entry) => filterEntry(entry.data, "b", "test", "success"))).toBeTruthy();
+    expect(jsonOutput.find((entry) => filterEntry(entry.data, "a", "build", "success"))).toBeTruthy();
+    expect(jsonOutput.find((entry) => filterEntry(entry.data, "a", "test", "success"))).toBeTruthy();
+
+    await repo.cleanup();
+  });
 });
diff --git a/packages/target-graph/src/removeNodes.ts b/packages/target-graph/src/removeNodes.ts
index 799885413..af5db5c85 100644
--- a/packages/target-graph/src/removeNodes.ts
+++ b/packages/target-graph/src/removeNodes.ts
@@ -1,5 +1,14 @@
 import type { Target } from "./types/Target.js";
 
+/**
+ * Remove nodes from the graph that are not runnable.
+ *
+ * When a node is removed, dependencies on that node are replaced with dependencies on the node's own
+ * dependencies.
+ *
+ * @param dag Node graph to process
+ * @param shouldDelete Predicate function identifying nodes to remove
+ */
 export async function removeNodes(dag: Target[], shouldDelete: (node: Target) => boolean | Promise<boolean>): Promise<Target[]> {
   // Create a map for quick lookup of nodes by id
   const nodeMap = new Map<string, Target>();
@@ -24,13 +33,20 @@ export async function removeNodes(dag: Target[], shouldDelete: (node: Target) =>
 
   // Update dependencies of remaining nodes
   for (const node of nodeMap.values()) {
-    const newDependencies = new Set(node.dependencies);
-    for (const depId of node.dependencies) {
-      if (additionalDependencies.has(depId)) {
-        additionalDependencies.get(depId)!.forEach((subDepId) => newDependencies.add(subDepId));
+    const newDependencies = new Set<string>();
+
+    // Inherit the dependencies of removed nodes we depended on
+    const visitDependency = (depId: string) => {
+      if (nodeMap.has(depId)) {
+        // It's still a valid node, keep in dependencies list
+        newDependencies.add(depId);
+      } else {
+        // Node was removed, propagate dependencies from the removed node
+        additionalDependencies.get(depId)?.forEach(visitDependency);
       }
-    }
-    node.dependencies = Array.from(newDependencies).filter((depId) => nodeMap.has(depId));
+    };
+    node.dependencies.forEach(visitDependency);
+    node.dependencies = Array.from(newDependencies);
   }
 
   // Convert the map back to an array

From 121536a0b44cf8b6ff2f99b9f81c8c7cc3311156 Mon Sep 17 00:00:00 2001
From: Dobes Vandermeer <dobes@formative.com>
Date: Fri, 18 Apr 2025 21:23:11 -0700
Subject: [PATCH 2/2] Change files

---
 .../change-8df26c07-7241-49f9-ab2b-72ef0e59f43f.json  | 11 +++++++++++
 1 file changed, 11 insertions(+)
 create mode 100644 change/change-8df26c07-7241-49f9-ab2b-72ef0e59f43f.json

diff --git a/change/change-8df26c07-7241-49f9-ab2b-72ef0e59f43f.json b/change/change-8df26c07-7241-49f9-ab2b-72ef0e59f43f.json
new file mode 100644
index 000000000..f0906a049
--- /dev/null
+++ b/change/change-8df26c07-7241-49f9-ab2b-72ef0e59f43f.json
@@ -0,0 +1,11 @@
+{
+  "changes": [
+    {
+      "type": "patch",
+      "comment": "Fix issue with noop tasks that depend on other noop tasks",
+      "packageName": "@lage-run/target-graph",
+      "email": "dobes@formative.com",
+      "dependentChangeType": "patch"
+    }
+  ]
+}
\ No newline at end of file