From 865070cff95a7de2d566428e3197601ee3a40ad6 Mon Sep 17 00:00:00 2001
From: Alon Navon <alon.n@sealsecurity.io>
Date: Thu, 23 Nov 2023 22:13:41 +0200
Subject: [PATCH 01/33] Settle overrides conflicts between edges, and propagate
 changes to the edges out

---
 workspaces/arborist/lib/edge.js |  4 +-
 workspaces/arborist/lib/node.js | 90 ++++++++++++++++++++++++++++++++-
 2 files changed, 90 insertions(+), 4 deletions(-)

diff --git a/workspaces/arborist/lib/edge.js b/workspaces/arborist/lib/edge.js
index cc9698ad6cae7..2a8b520509b01 100644
--- a/workspaces/arborist/lib/edge.js
+++ b/workspaces/arborist/lib/edge.js
@@ -258,7 +258,7 @@ class Edge {
     const newTo = this.#from.resolve(this.#name)
     if (newTo !== this.#to) {
       if (this.#to) {
-        this.#to.edgesIn.delete(this)
+        this.#to.deleteEdgeIn(this)
       }
       this.#to = newTo
       this.#error = null
@@ -273,7 +273,7 @@ class Edge {
   detach () {
     this.#explanation = null
     if (this.#to) {
-      this.#to.edgesIn.delete(this)
+      this.#to.deleteEdgeIn(this)
     }
     this.#from.edgesOut.delete(this.#name)
     this.#to = null
diff --git a/workspaces/arborist/lib/node.js b/workspaces/arborist/lib/node.js
index bdc021b7c12a9..975bcbad60861 100644
--- a/workspaces/arborist/lib/node.js
+++ b/workspaces/arborist/lib/node.js
@@ -1344,9 +1344,95 @@ class Node {
     this.edgesOut.set(edge.name, edge)
   }
 
-  addEdgeIn (edge) {
+  recalculateOutEdgesOverrides () {
+    // For each edge out propogate the new overrides through.
+    for (const edgePair of this.edgesOut) {
+      const edge = edgePair[1]
+      edge.reload(true)
+      if (edge.to) {
+        edge.to.updateNodeOverrideSet(edge.overrides)
+      }
+    }
+  }
+
+  findSpecificOverrideSet (first, second) {
+    for (let overrideSet = second; overrideSet; overrideSet = overrideSet.parent) {
+      if (overrideSet == first) {
+        return second
+      }
+    }
+    for (let overrideSet = first; overrideSet; overrideSet = overrideSet.parent) {
+      if (overrideSet == second) {
+        return first
+      }
+    }
+    return undefined
+  }
+
+  updateNodeOverrideSetDueToEdgeRemoval (otherOverrideSet) {
+    // If this edge's overrides isn't equal to this node's overrides, then removing it won't change newOverrideSet later.
+    if (this.overrides != otherOverrideSet) {
+      return false
+    }
+    let newOverrideSet
+    for (const edge of this.edgesIn) {
+      if (newOverrideSet) {
+        newOverrideSet = this.findSpecificOverrideSet()
+      } else {
+        newOverrideSet = edge.overrides
+      }
+    }
+    if (this.overrides == newOverrideSet) {
+      return false
+    }
+    this.overrides = newOverrideSet
+    this.recalculateOutEdgesOverrides()
+    return true
+  }
+
+  // This logic isn't perfect either. When we have two edges in that have different override sets, then we have to decide which set is correct.
+  // This function assumes the more specific override set is applicable, so if we have dependencies A->B->C and A->C
+  // and an override set that specifies what happens for C under A->B, this will work even if the new A->C edge comes along and tries to change
+  // the override set.
+  // The strictly correct logic is not to allow two edges with different overrides to point to the same node, because even if this node can satisfy
+  // both, one of its dependencies might need to be different depending on the edge leading to it.
+  // However, this might cause a lot of duplication, because the conflict in the dependencies might never actually happen.
+  updateNodeOverrideSet (otherOverrideSet) {
+    if (this.overrides == otherOverrideSet) {
+      return false
+    }
+    if (!this.overrides) {
+      this.overrides = otherOverrideSet
+      this.recalculateOutEdgesOverrides()
+      return true
+    }
+    let newOverrideSet = this.findSpecificOverrideSet(this.overrides, otherOverrideSet)
+    if (!newOverrideSet) {
+      // There are conflicting relevant override sets here. We should keep the remaining override set and mark the incoming edge as invalid somehow.
+      console.log("Overrides are in conflict.")
+    } else {
+      if (this.overrides != newOverrideSet) {
+        this.overrides = newOverrideSet
+        this.recalculateOutEdgesOverrides()
+        return true
+      }
+      return false
+    }
+  }
+  
+  deleteEdgeIn (edge) {
+    this.edgesIn.delete(edge)
     if (edge.overrides) {
-      this.overrides = edge.overrides
+      this.updateNodeOverrideSetDueToEdgeRemoval(edge.overrides)
+    }
+  }
+
+  addEdgeIn (edge) {
+    // We need to handle the case where the new edge in has an overrides field which is different from the current value.
+    // Assuming there are any overrides at all, the overrides field is never undefined for any node at the end state of the tree.
+    // So if the new edge's overrides is undefined it will be updated later. So we can wait with updating the node's overrides field.
+    if (edge.overrides && this.overrides != edge.overrides) {
+      this.updateNodeOverrideSet(edge.overrides)
     }
 
     this.edgesIn.add(edge)

From cd2019675427dd8d8cb5ffac1033447d014d141b Mon Sep 17 00:00:00 2001
From: Alon Navon <alon.n@sealsecurity.io>
Date: Sun, 26 Nov 2023 14:25:46 +0200
Subject: [PATCH 02/33] cleanup

---
 workspaces/arborist/lib/node.js | 6 ++----
 1 file changed, 2 insertions(+), 4 deletions(-)

diff --git a/workspaces/arborist/lib/node.js b/workspaces/arborist/lib/node.js
index 975bcbad60861..cffd9ed91b379 100644
--- a/workspaces/arborist/lib/node.js
+++ b/workspaces/arborist/lib/node.js
@@ -1346,8 +1346,7 @@ class Node {
 
   recalculateOutEdgesOverrides () {
     // For each edge out propogate the new overrides through.
-    for (const edgePair of this.edgesOut) {
-      const edge = edgePair[1]
+    for (const [, edge] of this.edgesOut) {
       edge.reload(true)
       if (edge.to) {
         edge.to.updateNodeOverrideSet(edge.overrides)
@@ -1366,7 +1365,6 @@ class Node {
         return first
       }
     }
-    return undefined
   }
 
   updateNodeOverrideSetDueToEdgeRemoval (otherOverrideSet) {
@@ -1409,7 +1407,7 @@ class Node {
     let newOverrideSet = this.findSpecificOverrideSet(this.overrides, otherOverrideSet)
     if (!newOverrideSet) {
       // There are conflicting relevant override sets here. We should keep the remaining override set and mark the incoming edge as invalid somehow.
-      console.log("Overrides are in conflict.")
+      throw Object.assign(new Error(`Two conflicting override sets for ${this.name}`), { code: 'EOVERRIDE' })
     } else {
       if (this.overrides != newOverrideSet) {
         this.overrides = newOverrideSet

From 0cad4e093ed917dada991eee21c02ae7809bc22b Mon Sep 17 00:00:00 2001
From: Alon Navon <alon.n@sealsecurity.io>
Date: Sun, 26 Nov 2023 15:35:55 +0200
Subject: [PATCH 03/33] Optimization

---
 workspaces/arborist/lib/node.js | 14 ++++++++++----
 1 file changed, 10 insertions(+), 4 deletions(-)

diff --git a/workspaces/arborist/lib/node.js b/workspaces/arborist/lib/node.js
index cffd9ed91b379..148a0f8aa8c91 100644
--- a/workspaces/arborist/lib/node.js
+++ b/workspaces/arborist/lib/node.js
@@ -1365,6 +1365,7 @@ class Node {
         return first
       }
     }
+    console.log("Conflicting override sets")
   }
 
   updateNodeOverrideSetDueToEdgeRemoval (otherOverrideSet) {
@@ -1375,7 +1376,7 @@ class Node {
     let newOverrideSet
     for (const edge of this.edgesIn) {
       if (newOverrideSet) {
-        newOverrideSet = this.findSpecificOverrideSet()
+        newOverrideSet = this.findSpecificOverrideSet(edge.overrides, newOverrideSet)
       } else {
         newOverrideSet = edge.overrides
       }
@@ -1384,7 +1385,12 @@ class Node {
       return false
     }
     this.overrides = newOverrideSet
-    this.recalculateOutEdgesOverrides()
+    if (this.overrides) {
+      // Optimization: if there's any override set at all, then no non-extraneous node has an empty override set. So if we temporarily have no
+      // override set (for example, we removed all the edges in), there's no use updating all the edges out right now. Let's just wait until
+      // we have an actual override set later.
+      this.recalculateOutEdgesOverrides()
+    }
     return true
   }
 
@@ -1406,8 +1412,8 @@ class Node {
     }
     let newOverrideSet = this.findSpecificOverrideSet(this.overrides, otherOverrideSet)
     if (!newOverrideSet) {
-      // There are conflicting relevant override sets here. We should keep the remaining override set and mark the incoming edge as invalid somehow.
-      throw Object.assign(new Error(`Two conflicting override sets for ${this.name}`), { code: 'EOVERRIDE' })
+      // This is an error condition. We can only get here if the new override set is in conflict with the existing.
+      return false
     } else {
       if (this.overrides != newOverrideSet) {
         this.overrides = newOverrideSet

From 6b276f3292ce0731e46e5af1b5d9eda3ed424edb Mon Sep 17 00:00:00 2001
From: Alon Navon <alon.n@sealsecurity.io>
Date: Tue, 28 Nov 2023 19:34:12 +0200
Subject: [PATCH 04/33] Fix override sets comparisons

---
 workspaces/arborist/lib/node.js | 14 +++++++-------
 1 file changed, 7 insertions(+), 7 deletions(-)

diff --git a/workspaces/arborist/lib/node.js b/workspaces/arborist/lib/node.js
index 148a0f8aa8c91..54895fead4637 100644
--- a/workspaces/arborist/lib/node.js
+++ b/workspaces/arborist/lib/node.js
@@ -1356,12 +1356,12 @@ class Node {
 
   findSpecificOverrideSet (first, second) {
     for (let overrideSet = second; overrideSet; overrideSet = overrideSet.parent) {
-      if (overrideSet == first) {
+      if (overrideSet.isEqual(first)) {
         return second
       }
     }
     for (let overrideSet = first; overrideSet; overrideSet = overrideSet.parent) {
-      if (overrideSet == second) {
+      if (overrideSet.isEqual(second)) {
         return first
       }
     }
@@ -1370,7 +1370,7 @@ class Node {
 
   updateNodeOverrideSetDueToEdgeRemoval (otherOverrideSet) {
     // If this edge's overrides isn't equal to this node's overrides, then removing it won't change newOverrideSet later.
-    if (this.overrides != otherOverrideSet) {
+    if (!this.overrides.isEqual(otherOverrideSet)) {
       return false
     }
     let newOverrideSet
@@ -1381,7 +1381,7 @@ class Node {
         newOverrideSet = edge.overrides
       }
     }
-    if (this.overrides == newOverrideSet) {
+    if (this.overrides.isEqual(newOverrideSet)) {
       return false
     }
     this.overrides = newOverrideSet
@@ -1402,7 +1402,7 @@ class Node {
   // both, one of its dependencies might need to be different depending on the edge leading to it.
   // However, this might cause a lot of duplication, because the conflict in the dependencies might never actually happen.
   updateNodeOverrideSet (otherOverrideSet) {
-    if (this.overrides == otherOverrideSet) {
+    if (this.overrides.isEqual(otherOverrideSet)) {
       return false
     }
     if (!this.overrides) {
@@ -1415,7 +1415,7 @@ class Node {
       // This is an error condition. We can only get here if the new override set is in conflict with the existing.
       return false
     } else {
-      if (this.overrides != newOverrideSet) {
+      if (!this.overrides.isEqual(newOverrideSet)) {
         this.overrides = newOverrideSet
         this.recalculateOutEdgesOverrides()
         return true
@@ -1435,7 +1435,7 @@ class Node {
     // We need to handle the case where the new edge in has an overrides field which is different from the current value.
     // Assuming there are any overrides at all, the overrides field is never undefined for any node at the end state of the tree.
     // So if the new edge's overrides is undefined it will be updated later. So we can wait with updating the node's overrides field.
-    if (edge.overrides && this.overrides != edge.overrides) {
+    if (edge.overrides && !this.overrides.isEqual(edge.overrides)) {
       this.updateNodeOverrideSet(edge.overrides)
     }
 

From fff30728d47a845647cb8541187e9ed41d1cbb4a Mon Sep 17 00:00:00 2001
From: Alon Navon <alon.n@sealsecurity.io>
Date: Tue, 28 Nov 2023 19:35:29 +0200
Subject: [PATCH 05/33] Add comparator

---
 workspaces/arborist/lib/override-set.js | 28 +++++++++++++++++++++++++
 1 file changed, 28 insertions(+)

diff --git a/workspaces/arborist/lib/override-set.js b/workspaces/arborist/lib/override-set.js
index bfc5a5d7906ee..b9f9b95293c8e 100644
--- a/workspaces/arborist/lib/override-set.js
+++ b/workspaces/arborist/lib/override-set.js
@@ -44,6 +44,34 @@ class OverrideSet {
     }
   }
 
+  childrenAreEqual (other) {
+    if (this.children.size !== other.children.size) {
+      return false
+    }
+    for (const [key, ] of this.children) {
+      if (!other.children.has(key)) {
+        return false
+      }
+      if (!this.children[key].value === other.children[key].value) {
+        return false
+      }
+      if (!this.children[key].childrenAreEqual(other.children[key])) {
+        return false
+      }
+    }
+    return true
+  }
+
+  isEqual (other) {
+    if (this === other) {
+      return true
+    }
+    if (this.key !== other.key || this.value !== other.value || !this.childrenAreEqual(other) || !this.parent.isEqual(other.parent)) {
+      return false
+    }
+    return true
+  }
+
   getEdgeRule (edge) {
     for (const rule of this.ruleset.values()) {
       if (rule.name !== edge.name) {

From e97176ea3b0ee9bbaacf05c549706005b0289f96 Mon Sep 17 00:00:00 2001
From: Alon Navon <alon.n@sealsecurity.io>
Date: Tue, 28 Nov 2023 19:45:18 +0200
Subject: [PATCH 06/33] Check for undefined

---
 workspaces/arborist/lib/node.js         | 13 ++++++++-----
 workspaces/arborist/lib/override-set.js |  3 +++
 2 files changed, 11 insertions(+), 5 deletions(-)

diff --git a/workspaces/arborist/lib/node.js b/workspaces/arborist/lib/node.js
index 54895fead4637..e5cdf2044ea4a 100644
--- a/workspaces/arborist/lib/node.js
+++ b/workspaces/arborist/lib/node.js
@@ -1370,7 +1370,7 @@ class Node {
 
   updateNodeOverrideSetDueToEdgeRemoval (otherOverrideSet) {
     // If this edge's overrides isn't equal to this node's overrides, then removing it won't change newOverrideSet later.
-    if (!this.overrides.isEqual(otherOverrideSet)) {
+    if (!this.overrides || !this.overrides.isEqual(otherOverrideSet)) {
       return false
     }
     let newOverrideSet
@@ -1402,13 +1402,16 @@ class Node {
   // both, one of its dependencies might need to be different depending on the edge leading to it.
   // However, this might cause a lot of duplication, because the conflict in the dependencies might never actually happen.
   updateNodeOverrideSet (otherOverrideSet) {
-    if (this.overrides.isEqual(otherOverrideSet)) {
-      return false
-    }
     if (!this.overrides) {
+      if (!otherOverrideSet) {
+        return false
+      }
       this.overrides = otherOverrideSet
       this.recalculateOutEdgesOverrides()
-      return true
+      return true      
+    }
+    if (this.overrides.isEqual(otherOverrideSet)) {
+      return false
     }
     let newOverrideSet = this.findSpecificOverrideSet(this.overrides, otherOverrideSet)
     if (!newOverrideSet) {
diff --git a/workspaces/arborist/lib/override-set.js b/workspaces/arborist/lib/override-set.js
index b9f9b95293c8e..c6069db9b46e0 100644
--- a/workspaces/arborist/lib/override-set.js
+++ b/workspaces/arborist/lib/override-set.js
@@ -66,6 +66,9 @@ class OverrideSet {
     if (this === other) {
       return true
     }
+    if (!other) {
+      return false
+    }
     if (this.key !== other.key || this.value !== other.value || !this.childrenAreEqual(other) || !this.parent.isEqual(other.parent)) {
       return false
     }

From 19155ed020f5c93a13a29ee4704709b65589cfc9 Mon Sep 17 00:00:00 2001
From: Alon Navon <alon.n@sealsecurity.io>
Date: Tue, 28 Nov 2023 19:49:40 +0200
Subject: [PATCH 07/33] Another place

---
 workspaces/arborist/lib/node.js | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/workspaces/arborist/lib/node.js b/workspaces/arborist/lib/node.js
index e5cdf2044ea4a..967a244b14c56 100644
--- a/workspaces/arborist/lib/node.js
+++ b/workspaces/arborist/lib/node.js
@@ -1438,7 +1438,7 @@ class Node {
     // We need to handle the case where the new edge in has an overrides field which is different from the current value.
     // Assuming there are any overrides at all, the overrides field is never undefined for any node at the end state of the tree.
     // So if the new edge's overrides is undefined it will be updated later. So we can wait with updating the node's overrides field.
-    if (edge.overrides && !this.overrides.isEqual(edge.overrides)) {
+    if (edge.overrides && (!this.overrides || !this.overrides.isEqual(edge.overrides))) {
       this.updateNodeOverrideSet(edge.overrides)
     }
 

From dedbccf213d44cca4bab0378651aeede16d7e49d Mon Sep 17 00:00:00 2001
From: Alon Navon <alon.n@sealsecurity.io>
Date: Tue, 28 Nov 2023 20:13:09 +0200
Subject: [PATCH 08/33] Fix

---
 workspaces/arborist/lib/override-set.js | 14 ++++++++++----
 1 file changed, 10 insertions(+), 4 deletions(-)

diff --git a/workspaces/arborist/lib/override-set.js b/workspaces/arborist/lib/override-set.js
index c6069db9b46e0..dcb928b60b216 100644
--- a/workspaces/arborist/lib/override-set.js
+++ b/workspaces/arborist/lib/override-set.js
@@ -52,10 +52,10 @@ class OverrideSet {
       if (!other.children.has(key)) {
         return false
       }
-      if (!this.children[key].value === other.children[key].value) {
+      if (!this.children.get(key).value === other.children.get(key).value) {
         return false
       }
-      if (!this.children[key].childrenAreEqual(other.children[key])) {
+      if (!this.children.get(key).childrenAreEqual(other.children.get(key))) {
         return false
       }
     }
@@ -69,10 +69,16 @@ class OverrideSet {
     if (!other) {
       return false
     }
-    if (this.key !== other.key || this.value !== other.value || !this.childrenAreEqual(other) || !this.parent.isEqual(other.parent)) {
+    if (this.key !== other.key || this.value !== other.value) {
       return false
     }
-    return true
+    if (!this.childrenAreEqual(other)) {
+      return false
+    }
+    if (!this.parent) {
+      return !other.parent
+    }
+    return this.parent.isEqual(other.parent)
   }
 
   getEdgeRule (edge) {

From 4a837861b0fd29b27502cfee2cf9dd72148e71c4 Mon Sep 17 00:00:00 2001
From: Alon Navon <alon.n@sealsecurity.io>
Date: Wed, 6 Dec 2023 15:48:51 +0200
Subject: [PATCH 09/33] Lint fixes

---
 workspaces/arborist/lib/node.js         | 8 ++++----
 workspaces/arborist/lib/override-set.js | 2 +-
 2 files changed, 5 insertions(+), 5 deletions(-)

diff --git a/workspaces/arborist/lib/node.js b/workspaces/arborist/lib/node.js
index 967a244b14c56..0533c852a67da 100644
--- a/workspaces/arborist/lib/node.js
+++ b/workspaces/arborist/lib/node.js
@@ -1365,7 +1365,7 @@ class Node {
         return first
       }
     }
-    console.log("Conflicting override sets")
+    console.log('Conflicting override sets')
   }
 
   updateNodeOverrideSetDueToEdgeRemoval (otherOverrideSet) {
@@ -1408,12 +1408,12 @@ class Node {
       }
       this.overrides = otherOverrideSet
       this.recalculateOutEdgesOverrides()
-      return true      
+      return true
     }
     if (this.overrides.isEqual(otherOverrideSet)) {
       return false
     }
-    let newOverrideSet = this.findSpecificOverrideSet(this.overrides, otherOverrideSet)
+    const newOverrideSet = this.findSpecificOverrideSet(this.overrides, otherOverrideSet)
     if (!newOverrideSet) {
       // This is an error condition. We can only get here if the new override set is in conflict with the existing.
       return false
@@ -1426,7 +1426,7 @@ class Node {
       return false
     }
   }
-  
+
   deleteEdgeIn (edge) {
     this.edgesIn.delete(edge)
     if (edge.overrides) {
diff --git a/workspaces/arborist/lib/override-set.js b/workspaces/arborist/lib/override-set.js
index dcb928b60b216..988a65d9e6bf8 100644
--- a/workspaces/arborist/lib/override-set.js
+++ b/workspaces/arborist/lib/override-set.js
@@ -48,7 +48,7 @@ class OverrideSet {
     if (this.children.size !== other.children.size) {
       return false
     }
-    for (const [key, ] of this.children) {
+    for (const [key,] of this.children) {
       if (!other.children.has(key)) {
         return false
       }

From 7ee2f2e668bd327ba39fcc61bf90ba07f7dae9cf Mon Sep 17 00:00:00 2001
From: Alon Navon <alon.n@sealsecurity.io>
Date: Wed, 6 Dec 2023 19:21:28 +0200
Subject: [PATCH 10/33] Added tests

---
 workspaces/arborist/lib/node.js               |  12 +-
 workspaces/arborist/lib/override-set.js       |   4 +-
 .../tap-snapshots/test/edge.js.test.cjs       |  44 +++++++
 workspaces/arborist/test/edge.js              | 123 ++++++++++++++++++
 4 files changed, 174 insertions(+), 9 deletions(-)

diff --git a/workspaces/arborist/lib/node.js b/workspaces/arborist/lib/node.js
index 0533c852a67da..d6702bffab388 100644
--- a/workspaces/arborist/lib/node.js
+++ b/workspaces/arborist/lib/node.js
@@ -1403,6 +1403,8 @@ class Node {
   // However, this might cause a lot of duplication, because the conflict in the dependencies might never actually happen.
   updateNodeOverrideSet (otherOverrideSet) {
     if (!this.overrides) {
+      // Assuming there are any overrides at all, the overrides field is never undefined for any node at the end state of the tree.
+      // So if the new edge's overrides is undefined it will be updated later. So we can wait with updating the node's overrides field.
       if (!otherOverrideSet) {
         return false
       }
@@ -1414,10 +1416,7 @@ class Node {
       return false
     }
     const newOverrideSet = this.findSpecificOverrideSet(this.overrides, otherOverrideSet)
-    if (!newOverrideSet) {
-      // This is an error condition. We can only get here if the new override set is in conflict with the existing.
-      return false
-    } else {
+    if (newOverrideSet) {
       if (!this.overrides.isEqual(newOverrideSet)) {
         this.overrides = newOverrideSet
         this.recalculateOutEdgesOverrides()
@@ -1425,6 +1424,7 @@ class Node {
       }
       return false
     }
+    // This is an error condition. We can only get here if the new override set is in conflict with the existing.
   }
 
   deleteEdgeIn (edge) {
@@ -1436,9 +1436,7 @@ class Node {
 
   addEdgeIn (edge) {
     // We need to handle the case where the new edge in has an overrides field which is different from the current value.
-    // Assuming there are any overrides at all, the overrides field is never undefined for any node at the end state of the tree.
-    // So if the new edge's overrides is undefined it will be updated later. So we can wait with updating the node's overrides field.
-    if (edge.overrides && (!this.overrides || !this.overrides.isEqual(edge.overrides))) {
+    if (!this.overrides || !this.overrides.isEqual(edge.overrides)) {
       this.updateNodeOverrideSet(edge.overrides)
     }
 
diff --git a/workspaces/arborist/lib/override-set.js b/workspaces/arborist/lib/override-set.js
index 988a65d9e6bf8..a03ed5133aa59 100644
--- a/workspaces/arborist/lib/override-set.js
+++ b/workspaces/arborist/lib/override-set.js
@@ -48,11 +48,11 @@ class OverrideSet {
     if (this.children.size !== other.children.size) {
       return false
     }
-    for (const [key,] of this.children) {
+    for (const [key] of this.children) {
       if (!other.children.has(key)) {
         return false
       }
-      if (!this.children.get(key).value === other.children.get(key).value) {
+      if (this.children.get(key).value !== other.children.get(key).value) {
         return false
       }
       if (!this.children.get(key).childrenAreEqual(other.children.get(key))) {
diff --git a/workspaces/arborist/tap-snapshots/test/edge.js.test.cjs b/workspaces/arborist/tap-snapshots/test/edge.js.test.cjs
index 17dc0b0c9fb0b..7b28779165d56 100644
--- a/workspaces/arborist/tap-snapshots/test/edge.js.test.cjs
+++ b/workspaces/arborist/tap-snapshots/test/edge.js.test.cjs
@@ -52,6 +52,7 @@ Edge {
   "from": Object {
     "addEdgeIn": Function addEdgeIn(edge),
     "addEdgeOut": Function addEdgeOut(edge),
+    "deleteEdgeIn": Function deleteEdgeIn(edge),
     "edgesIn": Set {},
     "edgesOut": Map {
       "b" => Edge {
@@ -69,6 +70,7 @@ Edge {
     "parent": Object {
       "addEdgeIn": Function addEdgeIn(edge),
       "addEdgeOut": Function addEdgeOut(edge),
+      "deleteEdgeIn": Function deleteEdgeIn(edge),
       "edgesIn": Set {},
       "edgesOut": Map {
         "missing" => Edge {
@@ -91,6 +93,7 @@ Edge {
     "root": Object {
       "addEdgeIn": Function addEdgeIn(edge),
       "addEdgeOut": Function addEdgeOut(edge),
+      "deleteEdgeIn": Function deleteEdgeIn(edge),
       "edgesIn": Set {},
       "edgesOut": Map {
         "missing" => Edge {
@@ -122,6 +125,7 @@ Edge {
   "to": Object {
     "addEdgeIn": Function addEdgeIn(edge),
     "addEdgeOut": Function addEdgeOut(edge),
+    "deleteEdgeIn": Function deleteEdgeIn(edge),
     "edgesIn": Set {
       Edge {
         "peerConflicted": false,
@@ -139,6 +143,7 @@ Edge {
     "parent": Object {
       "addEdgeIn": Function addEdgeIn(edge),
       "addEdgeOut": Function addEdgeOut(edge),
+      "deleteEdgeIn": Function deleteEdgeIn(edge),
       "edgesIn": Set {},
       "edgesOut": Map {
         "missing" => Edge {
@@ -173,6 +178,7 @@ Edge {
   "from": Object {
     "addEdgeIn": Function addEdgeIn(edge),
     "addEdgeOut": Function addEdgeOut(edge),
+    "deleteEdgeIn": Function deleteEdgeIn(edge),
     "edgesIn": Set {},
     "edgesOut": Map {
       "b" => Edge {
@@ -190,6 +196,7 @@ Edge {
     "parent": Object {
       "addEdgeIn": Function addEdgeIn(edge),
       "addEdgeOut": Function addEdgeOut(edge),
+      "deleteEdgeIn": Function deleteEdgeIn(edge),
       "edgesIn": Set {},
       "edgesOut": Map {
         "missing" => Edge {
@@ -212,6 +219,7 @@ Edge {
     "root": Object {
       "addEdgeIn": Function addEdgeIn(edge),
       "addEdgeOut": Function addEdgeOut(edge),
+      "deleteEdgeIn": Function deleteEdgeIn(edge),
       "edgesIn": Set {},
       "edgesOut": Map {
         "missing" => Edge {
@@ -243,6 +251,7 @@ Edge {
   "to": Object {
     "addEdgeIn": Function addEdgeIn(edge),
     "addEdgeOut": Function addEdgeOut(edge),
+    "deleteEdgeIn": Function deleteEdgeIn(edge),
     "edgesIn": Set {
       Edge {
         "peerConflicted": false,
@@ -260,6 +269,7 @@ Edge {
     "parent": Object {
       "addEdgeIn": Function addEdgeIn(edge),
       "addEdgeOut": Function addEdgeOut(edge),
+      "deleteEdgeIn": Function deleteEdgeIn(edge),
       "edgesIn": Set {},
       "edgesOut": Map {
         "missing" => Edge {
@@ -294,6 +304,7 @@ Edge {
   "from": Object {
     "addEdgeIn": Function addEdgeIn(edge),
     "addEdgeOut": Function addEdgeOut(edge),
+    "deleteEdgeIn": Function deleteEdgeIn(edge),
     "edgesIn": Set {},
     "edgesOut": Map {
       "missing" => Edge {
@@ -334,6 +345,7 @@ Edge {
   "from": Object {
     "addEdgeIn": Function addEdgeIn(edge),
     "addEdgeOut": Function addEdgeOut(edge),
+    "deleteEdgeIn": Function deleteEdgeIn(edge),
     "edgesIn": Set {},
     "edgesOut": Map {
       "aa" => Edge {
@@ -351,6 +363,7 @@ Edge {
     "parent": Object {
       "addEdgeIn": Function addEdgeIn(edge),
       "addEdgeOut": Function addEdgeOut(edge),
+      "deleteEdgeIn": Function deleteEdgeIn(edge),
       "edgesIn": Set {},
       "edgesOut": Map {
         "b" => Edge {
@@ -368,6 +381,7 @@ Edge {
       "parent": Object {
         "addEdgeIn": Function addEdgeIn(edge),
         "addEdgeOut": Function addEdgeOut(edge),
+        "deleteEdgeIn": Function deleteEdgeIn(edge),
         "edgesIn": Set {},
         "edgesOut": Map {
           "missing" => Edge {
@@ -390,6 +404,7 @@ Edge {
       "root": Object {
         "addEdgeIn": Function addEdgeIn(edge),
         "addEdgeOut": Function addEdgeOut(edge),
+        "deleteEdgeIn": Function deleteEdgeIn(edge),
         "edgesIn": Set {},
         "edgesOut": Map {
           "missing" => Edge {
@@ -424,6 +439,7 @@ Edge {
   "to": Object {
     "addEdgeIn": Function addEdgeIn(edge),
     "addEdgeOut": Function addEdgeOut(edge),
+    "deleteEdgeIn": Function deleteEdgeIn(edge),
     "edgesIn": Set {
       Edge {
         "peerConflicted": false,
@@ -441,6 +457,7 @@ Edge {
     "parent": Object {
       "addEdgeIn": Function addEdgeIn(edge),
       "addEdgeOut": Function addEdgeOut(edge),
+      "deleteEdgeIn": Function deleteEdgeIn(edge),
       "edgesIn": Set {},
       "edgesOut": Map {
         "b" => Edge {
@@ -458,6 +475,7 @@ Edge {
       "parent": Object {
         "addEdgeIn": Function addEdgeIn(edge),
         "addEdgeOut": Function addEdgeOut(edge),
+        "deleteEdgeIn": Function deleteEdgeIn(edge),
         "edgesIn": Set {},
         "edgesOut": Map {
           "missing" => Edge {
@@ -480,6 +498,7 @@ Edge {
       "root": Object {
         "addEdgeIn": Function addEdgeIn(edge),
         "addEdgeOut": Function addEdgeOut(edge),
+        "deleteEdgeIn": Function deleteEdgeIn(edge),
         "edgesIn": Set {},
         "edgesOut": Map {
           "missing" => Edge {
@@ -516,6 +535,7 @@ Edge {
   "from": Object {
     "addEdgeIn": Function addEdgeIn(edge),
     "addEdgeOut": Function addEdgeOut(edge),
+    "deleteEdgeIn": Function deleteEdgeIn(edge),
     "edgesIn": Set {},
     "edgesOut": Map {
       "aa" => Edge {
@@ -533,6 +553,7 @@ Edge {
     "parent": Object {
       "addEdgeIn": Function addEdgeIn(edge),
       "addEdgeOut": Function addEdgeOut(edge),
+      "deleteEdgeIn": Function deleteEdgeIn(edge),
       "edgesIn": Set {},
       "edgesOut": Map {
         "missing" => Edge {
@@ -576,6 +597,7 @@ Edge {
   "from": Object {
     "addEdgeIn": Function addEdgeIn(edge),
     "addEdgeOut": Function addEdgeOut(edge),
+    "deleteEdgeIn": Function deleteEdgeIn(edge),
     "edgesIn": Set {},
     "edgesOut": Map {
       "aa" => Edge {
@@ -593,6 +615,7 @@ Edge {
     "parent": Object {
       "addEdgeIn": Function addEdgeIn(edge),
       "addEdgeOut": Function addEdgeOut(edge),
+      "deleteEdgeIn": Function deleteEdgeIn(edge),
       "edgesIn": Set {
         Edge {
           "peerConflicted": false,
@@ -610,6 +633,7 @@ Edge {
       "parent": Object {
         "addEdgeIn": Function addEdgeIn(edge),
         "addEdgeOut": Function addEdgeOut(edge),
+        "deleteEdgeIn": Function deleteEdgeIn(edge),
         "edgesIn": Set {},
         "edgesOut": Map {
           "missing" => Edge {
@@ -645,6 +669,7 @@ Edge {
   "to": Object {
     "addEdgeIn": Function addEdgeIn(edge),
     "addEdgeOut": Function addEdgeOut(edge),
+    "deleteEdgeIn": Function deleteEdgeIn(edge),
     "edgesIn": Set {
       Edge {
         "peerConflicted": false,
@@ -662,6 +687,7 @@ Edge {
     "parent": Object {
       "addEdgeIn": Function addEdgeIn(edge),
       "addEdgeOut": Function addEdgeOut(edge),
+      "deleteEdgeIn": Function deleteEdgeIn(edge),
       "edgesIn": Set {},
       "edgesOut": Map {
         "b" => Edge {
@@ -679,6 +705,7 @@ Edge {
       "parent": Object {
         "addEdgeIn": Function addEdgeIn(edge),
         "addEdgeOut": Function addEdgeOut(edge),
+        "deleteEdgeIn": Function deleteEdgeIn(edge),
         "edgesIn": Set {},
         "edgesOut": Map {
           "missing" => Edge {
@@ -701,6 +728,7 @@ Edge {
       "root": Object {
         "addEdgeIn": Function addEdgeIn(edge),
         "addEdgeOut": Function addEdgeOut(edge),
+        "deleteEdgeIn": Function deleteEdgeIn(edge),
         "edgesIn": Set {},
         "edgesOut": Map {
           "missing" => Edge {
@@ -737,6 +765,7 @@ Edge {
   "from": Object {
     "addEdgeIn": Function addEdgeIn(edge),
     "addEdgeOut": Function addEdgeOut(edge),
+    "deleteEdgeIn": Function deleteEdgeIn(edge),
     "edgesIn": Set {},
     "edgesOut": Map {
       "a" => Edge {
@@ -766,6 +795,7 @@ Edge {
   "to": Object {
     "addEdgeIn": Function addEdgeIn(edge),
     "addEdgeOut": Function addEdgeOut(edge),
+    "deleteEdgeIn": Function deleteEdgeIn(edge),
     "edgesIn": Set {
       Edge {
         "peerConflicted": false,
@@ -783,6 +813,7 @@ Edge {
     "parent": Object {
       "addEdgeIn": Function addEdgeIn(edge),
       "addEdgeOut": Function addEdgeOut(edge),
+      "deleteEdgeIn": Function deleteEdgeIn(edge),
       "edgesIn": Set {},
       "edgesOut": Map {
         "a" => Edge {
@@ -805,6 +836,7 @@ Edge {
     "root": Object {
       "addEdgeIn": Function addEdgeIn(edge),
       "addEdgeOut": Function addEdgeOut(edge),
+      "deleteEdgeIn": Function deleteEdgeIn(edge),
       "edgesIn": Set {},
       "edgesOut": Map {
         "a" => Edge {
@@ -838,6 +870,7 @@ Edge {
   "from": Object {
     "addEdgeIn": Function addEdgeIn(edge),
     "addEdgeOut": Function addEdgeOut(edge),
+    "deleteEdgeIn": Function deleteEdgeIn(edge),
     "edgesIn": Set {},
     "edgesOut": Map {
       "aa" => Edge {
@@ -855,6 +888,7 @@ Edge {
     "parent": Object {
       "addEdgeIn": Function addEdgeIn(edge),
       "addEdgeOut": Function addEdgeOut(edge),
+      "deleteEdgeIn": Function deleteEdgeIn(edge),
       "edgesIn": Set {},
       "edgesOut": Map {
         "missing" => Edge {
@@ -877,6 +911,7 @@ Edge {
     "root": Object {
       "addEdgeIn": Function addEdgeIn(edge),
       "addEdgeOut": Function addEdgeOut(edge),
+      "deleteEdgeIn": Function deleteEdgeIn(edge),
       "edgesIn": Set {},
       "edgesOut": Map {
         "missing" => Edge {
@@ -908,6 +943,7 @@ Edge {
   "to": Object {
     "addEdgeIn": Function addEdgeIn(edge),
     "addEdgeOut": Function addEdgeOut(edge),
+    "deleteEdgeIn": Function deleteEdgeIn(edge),
     "edgesIn": Set {
       Edge {
         "peerConflicted": false,
@@ -925,6 +961,7 @@ Edge {
     "parent": Object {
       "addEdgeIn": Function addEdgeIn(edge),
       "addEdgeOut": Function addEdgeOut(edge),
+      "deleteEdgeIn": Function deleteEdgeIn(edge),
       "edgesIn": Set {},
       "edgesOut": Map {
         "aa" => Edge {
@@ -942,6 +979,7 @@ Edge {
       "parent": Object {
         "addEdgeIn": Function addEdgeIn(edge),
         "addEdgeOut": Function addEdgeOut(edge),
+        "deleteEdgeIn": Function deleteEdgeIn(edge),
         "edgesIn": Set {},
         "edgesOut": Map {
           "missing" => Edge {
@@ -964,6 +1002,7 @@ Edge {
       "root": Object {
         "addEdgeIn": Function addEdgeIn(edge),
         "addEdgeOut": Function addEdgeOut(edge),
+        "deleteEdgeIn": Function deleteEdgeIn(edge),
         "edgesIn": Set {},
         "edgesOut": Map {
           "missing" => Edge {
@@ -1000,6 +1039,7 @@ Edge {
   "from": Object {
     "addEdgeIn": Function addEdgeIn(edge),
     "addEdgeOut": Function addEdgeOut(edge),
+    "deleteEdgeIn": Function deleteEdgeIn(edge),
     "edgesIn": Set {},
     "edgesOut": Map {
       "b" => Edge {
@@ -1017,6 +1057,7 @@ Edge {
     "parent": Object {
       "addEdgeIn": Function addEdgeIn(edge),
       "addEdgeOut": Function addEdgeOut(edge),
+      "deleteEdgeIn": Function deleteEdgeIn(edge),
       "edgesIn": Set {},
       "edgesOut": Map {
         "missing" => Edge {
@@ -1039,6 +1080,7 @@ Edge {
     "root": Object {
       "addEdgeIn": Function addEdgeIn(edge),
       "addEdgeOut": Function addEdgeOut(edge),
+      "deleteEdgeIn": Function deleteEdgeIn(edge),
       "edgesIn": Set {},
       "edgesOut": Map {
         "missing" => Edge {
@@ -1070,6 +1112,7 @@ Edge {
   "to": Object {
     "addEdgeIn": Function addEdgeIn(edge),
     "addEdgeOut": Function addEdgeOut(edge),
+    "deleteEdgeIn": Function deleteEdgeIn(edge),
     "edgesIn": Set {
       Edge {
         "peerConflicted": false,
@@ -1087,6 +1130,7 @@ Edge {
     "parent": Object {
       "addEdgeIn": Function addEdgeIn(edge),
       "addEdgeOut": Function addEdgeOut(edge),
+      "deleteEdgeIn": Function deleteEdgeIn(edge),
       "edgesIn": Set {},
       "edgesOut": Map {
         "missing" => Edge {
diff --git a/workspaces/arborist/test/edge.js b/workspaces/arborist/test/edge.js
index ab08357ece359..9a2e5791b4de1 100644
--- a/workspaces/arborist/test/edge.js
+++ b/workspaces/arborist/test/edge.js
@@ -57,6 +57,9 @@ const top = {
   addEdgeIn (edge) {
     this.edgesIn.add(edge)
   },
+  deleteEdgeIn (edge) {
+    this.edgesIn.delete(edge)
+  },
 }
 
 const a = {
@@ -81,6 +84,9 @@ const a = {
   addEdgeIn (edge) {
     this.edgesIn.add(edge)
   },
+  deleteEdgeIn (edge) {
+    this.edgesIn.delete(edge)
+  },
 }
 
 const b = {
@@ -104,6 +110,9 @@ const b = {
   addEdgeIn (edge) {
     this.edgesIn.add(edge)
   },
+  deleteEdgeIn (edge) {
+    this.edgesIn.delete(edge)
+  },
 }
 
 const bb = {
@@ -127,6 +136,9 @@ const bb = {
   addEdgeIn (edge) {
     this.edgesIn.add(edge)
   },
+  deleteEdgeIn (edge) {
+    this.edgesIn.delete(edge)
+  },
 }
 
 const aa = {
@@ -150,6 +162,9 @@ const aa = {
   addEdgeIn (edge) {
     this.edgesIn.add(edge)
   },
+  deleteEdgeIn (edge) {
+    this.edgesIn.delete(edge)
+  },
 }
 
 const c = {
@@ -173,6 +188,9 @@ const c = {
   addEdgeIn (edge) {
     this.edgesIn.add(edge)
   },
+  deleteEdgeIn (edge) {
+    this.edgesIn.delete(edge)
+  },
 }
 
 t.matchSnapshot(new Edge({
@@ -364,6 +382,9 @@ const referenceTop = {
   addEdgeIn (edge) {
     this.edgesIn.add(edge)
   },
+  deleteEdgeIn (edge) {
+    this.edgesIn.delete(edge)
+  },
   overrides: new OverrideSet({
     overrides: {
       referenceGrandchild: '$referenceChild',
@@ -403,6 +424,9 @@ const referenceChild = {
     this.overrides = edge.overrides
     this.edgesIn.add(edge)
   },
+  deleteEdgeIn (edge) {
+    this.edgesIn.delete(edge)
+  },
 }
 
 new Edge({
@@ -442,6 +466,9 @@ const referenceGrandchild = {
     this.overrides = edge.overrides
     this.edgesIn.add(edge)
   },
+  deleteEdgeIn (edge) {
+    this.edgesIn.delete(edge)
+  },
 }
 
 const referenceGrandchildEdge = new Edge({
@@ -490,6 +517,9 @@ const badOverride = {
   addEdgeIn (edge) {
     this.edgesIn.add(edge)
   },
+  deleteEdgeIn (edge) {
+    this.edgesIn.delete(edge)
+  },
   overrides: new OverrideSet({
     overrides: {
       b: '1.x',
@@ -775,6 +805,9 @@ const bundleChild = {
   addEdgeIn (edge) {
     this.edgesIn.add(edge)
   },
+  deleteEdgeIn (edge) {
+    this.edgesIn.delete(edge)
+  },
 }
 
 const bundleParent = {
@@ -797,6 +830,9 @@ const bundleParent = {
   addEdgeIn (edge) {
     this.edgesIn.add(edge)
   },
+  deleteEdgeIn (edge) {
+    this.edgesIn.delete(edge)
+  },
 }
 
 const bundledEdge = new Edge({
@@ -858,6 +894,9 @@ t.test('override references find the correct root', (t) => {
     addEdgeIn (edge) {
       this.edgesIn.add(edge)
     },
+    deleteEdgeIn (edge) {
+      this.edgesIn.delete(edge)
+    },
   }
 
   const foo = {
@@ -885,6 +924,9 @@ t.test('override references find the correct root', (t) => {
     addEdgeIn (edge) {
       this.edgesIn.add(edge)
     },
+    deleteEdgeIn (edge) {
+      this.edgesIn.delete(edge)
+    },
   }
   foo.overrides = overrides.getNodeRule(foo)
 
@@ -915,6 +957,9 @@ t.test('override references find the correct root', (t) => {
     addEdgeIn (edge) {
       this.edgesIn.add(edge)
     },
+    deleteEdgeIn (edge) {
+      this.edgesIn.delete(edge)
+    },
   }
   bar.overrides = foo.overrides.getNodeRule(bar)
 
@@ -946,6 +991,9 @@ t.test('override references find the correct root', (t) => {
     addEdgeIn (edge) {
       this.edgesIn.add(edge)
     },
+    deleteEdgeIn (edge) {
+      this.edgesIn.delete(edge)
+    },
   }
   virtualBar.overrides = overrides
 
@@ -999,6 +1047,9 @@ t.test('shrinkwrapped and bundled deps are not overridden and remain valid', (t)
     addEdgeIn (edge) {
       this.edgesIn.add(edge)
     },
+    deleteEdgeIn (edge) {
+      this.edgesIn.delete(edge)
+    },
   }
 
   const foo = {
@@ -1029,6 +1080,9 @@ t.test('shrinkwrapped and bundled deps are not overridden and remain valid', (t)
     addEdgeIn (edge) {
       this.edgesIn.add(edge)
     },
+    deleteEdgeIn (edge) {
+      this.edgesIn.delete(edge)
+    },
   }
   foo.overrides = overrides.getNodeRule(foo)
 
@@ -1058,6 +1112,9 @@ t.test('shrinkwrapped and bundled deps are not overridden and remain valid', (t)
     addEdgeIn (edge) {
       this.edgesIn.add(edge)
     },
+    deleteEdgeIn (edge) {
+      this.edgesIn.delete(edge)
+    },
   }
   bar.overrides = foo.overrides.getNodeRule(bar)
 
@@ -1072,3 +1129,69 @@ t.test('shrinkwrapped and bundled deps are not overridden and remain valid', (t)
   t.ok(edge.valid, 'edge is valid')
   t.end()
 })
+
+t.test('overrideset comparison logic', (t) => {
+  const overrides1 = new OverrideSet({
+    overrides: {
+      bar: '^2.0.0',
+    },
+  })
+
+  const overrides2 = new OverrideSet({
+    overrides: {
+      bar: '^2.0.0',
+    },
+  })
+
+  const overrides3 = new OverrideSet({
+    overrides: {
+      foo: '^2.0.0',
+    },
+  })
+
+  const overrides4 = new OverrideSet({
+    overrides: {
+      foo: '^1.0.0',
+    },
+  })
+
+  const overrides5 = new OverrideSet({
+    overrides: {
+      bar: '^2.0.0',
+      foo: '^2.0.0',
+    },
+  })
+
+  const overrides6 = new OverrideSet({
+    overrides: {
+    },
+  })
+
+  const overrides7 = new OverrideSet({
+    overrides: {
+      bar: {
+        '.': '^2.0.0',
+        baz: '1.2.3',
+      },
+    },
+  })
+
+  t.ok(overrides1.isEqual(overrides1), 'overridesets are equal')
+  t.ok(overrides1.isEqual(overrides2), 'overridesets are equal')
+  t.ok(!overrides1.isEqual(overrides3), 'overridesets are different')
+  t.ok(!overrides1.isEqual(overrides5), 'overridesets are different')
+  t.ok(!overrides1.isEqual(overrides6), 'overridesets are different')
+  t.ok(!overrides1.isEqual(overrides7), 'overridesets are different')
+  t.ok(!overrides3.isEqual(overrides1), 'overridesets are different')
+  t.ok(!overrides3.isEqual(overrides4), 'overridesets are different')
+  t.ok(!overrides3.isEqual(overrides5), 'overridesets are different')
+  t.ok(!overrides4.isEqual(overrides5), 'overridesets are different')
+  t.ok(!overrides5.isEqual(overrides1), 'overridesets are different')
+  t.ok(!overrides5.isEqual(overrides3), 'overridesets are different')
+  t.ok(!overrides5.isEqual(overrides6), 'overridesets are different')
+  t.ok(!overrides6.isEqual(overrides1), 'overridesets are different')
+  t.ok(!overrides6.isEqual(overrides3), 'overridesets are different')
+  t.ok(overrides6.isEqual(overrides6), 'overridesets are equal')
+  t.ok(!overrides7.isEqual(overrides1), 'overridesets are different')
+  t.end()
+})

From 091742184d3be69694ce93bbbbc3a5077202f019 Mon Sep 17 00:00:00 2001
From: Alon Navon <alon.n@sealsecurity.io>
Date: Sun, 27 Oct 2024 21:29:38 +0200
Subject: [PATCH 11/33] Overrides aren't inherited from parent, but from to
 nodes

---
 workspaces/arborist/lib/node.js | 4 ----
 1 file changed, 4 deletions(-)

diff --git a/workspaces/arborist/lib/node.js b/workspaces/arborist/lib/node.js
index d6702bffab388..ed43b50e4bc9b 100644
--- a/workspaces/arborist/lib/node.js
+++ b/workspaces/arborist/lib/node.js
@@ -1247,10 +1247,6 @@ class Node {
       this[_changePath](newPath)
     }
 
-    if (parent.overrides) {
-      this.overrides = parent.overrides.getNodeRule(this)
-    }
-
     // clobbers anything at that path, resets all appropriate references
     this.root = parent.root
   }

From 6d0ad42d5320871d2708cdb80ba00b263a923f9d Mon Sep 17 00:00:00 2001
From: Alon Navon <alon.n@sealsecurity.io>
Date: Mon, 28 Oct 2024 23:26:43 +0200
Subject: [PATCH 12/33] Always use raw spec when analyzing overrides

---
 workspaces/arborist/lib/override-set.js | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/workspaces/arborist/lib/override-set.js b/workspaces/arborist/lib/override-set.js
index a03ed5133aa59..8a64333204f9a 100644
--- a/workspaces/arborist/lib/override-set.js
+++ b/workspaces/arborist/lib/override-set.js
@@ -92,7 +92,7 @@ class OverrideSet {
         return rule
       }
 
-      let spec = npa(`${edge.name}@${edge.spec}`)
+      let spec = npa(`${edge.name}@${edge.rawSpec}`)
       if (spec.type === 'alias') {
         spec = spec.subSpec
       }

From 67d7d5cfecc844ef990ca53c2134a10cbde840d4 Mon Sep 17 00:00:00 2001
From: Alon Navon <alon.n@sealsecurity.io>
Date: Mon, 28 Oct 2024 23:28:48 +0200
Subject: [PATCH 13/33] Replaceability

---
 workspaces/arborist/lib/node.js | 15 ++++++++++++---
 1 file changed, 12 insertions(+), 3 deletions(-)

diff --git a/workspaces/arborist/lib/node.js b/workspaces/arborist/lib/node.js
index ed43b50e4bc9b..8d5c7c7cfdad9 100644
--- a/workspaces/arborist/lib/node.js
+++ b/workspaces/arborist/lib/node.js
@@ -1004,10 +1004,19 @@ class Node {
       return false
     }
 
-    // XXX need to check for two root nodes?
-    if (node.overrides !== this.overrides) {
-      return false
+    if (this.edgesOut.size) {
+      // XXX need to check for two root nodes?
+      if (node.overrides) {
+        if (!node.overrides.isEqual(this.overrides)) {
+          return false
+        }
+      } else {
+        if (this.overrides) {
+          return false
+        }
+      }
     }
+
     ignorePeers = new Set(ignorePeers)
 
     // gather up all the deps of this node and that are only depended

From 643cbeac9e7fe47242026af09dfc92902b701943 Mon Sep 17 00:00:00 2001
From: Alon Navon <alon.n@sealsecurity.io>
Date: Mon, 28 Oct 2024 23:30:00 +0200
Subject: [PATCH 14/33] Overridden status

---
 workspaces/arborist/lib/node.js | 19 ++++++++++++++++++-
 1 file changed, 18 insertions(+), 1 deletion(-)

diff --git a/workspaces/arborist/lib/node.js b/workspaces/arborist/lib/node.js
index 8d5c7c7cfdad9..dcb30fbac5b5b 100644
--- a/workspaces/arborist/lib/node.js
+++ b/workspaces/arborist/lib/node.js
@@ -342,7 +342,24 @@ class Node {
   }
 
   get overridden () {
-    return !!(this.overrides && this.overrides.value && this.overrides.name === this.name)
+    if (!this.overrides) {
+      return false
+    }
+    if (!this.overrides.value) {
+      return false
+    }
+    if (this.overrides.name !== this.name) {
+      return false
+    }
+    for (const edge of this.edgesIn) {
+      if (this.overrides.isEqual(edge.overrides)) {
+        if (!edge.overrides.isEqual(edge.from.overrides)) {
+          return true
+        }
+      }
+    }
+
+    return false
   }
 
   get package () {

From c5f6e4af79ca8d3d9be1d1bd93f5150c73bfadcb Mon Sep 17 00:00:00 2001
From: Alon Navon <alon.n@sealsecurity.io>
Date: Mon, 28 Oct 2024 23:30:42 +0200
Subject: [PATCH 15/33] Fix undefined bugs

---
 workspaces/arborist/lib/node.js | 10 +++++-----
 1 file changed, 5 insertions(+), 5 deletions(-)

diff --git a/workspaces/arborist/lib/node.js b/workspaces/arborist/lib/node.js
index dcb30fbac5b5b..5ec074e8766b1 100644
--- a/workspaces/arborist/lib/node.js
+++ b/workspaces/arborist/lib/node.js
@@ -1397,7 +1397,7 @@ class Node {
     }
     let newOverrideSet
     for (const edge of this.edgesIn) {
-      if (newOverrideSet) {
+      if (newOverrideSet && edge.overrides) {
         newOverrideSet = this.findSpecificOverrideSet(edge.overrides, newOverrideSet)
       } else {
         newOverrideSet = edge.overrides
@@ -1424,12 +1424,12 @@ class Node {
   // both, one of its dependencies might need to be different depending on the edge leading to it.
   // However, this might cause a lot of duplication, because the conflict in the dependencies might never actually happen.
   updateNodeOverrideSet (otherOverrideSet) {
-    if (!this.overrides) {
+    if (!otherOverrideSet) {
       // Assuming there are any overrides at all, the overrides field is never undefined for any node at the end state of the tree.
       // So if the new edge's overrides is undefined it will be updated later. So we can wait with updating the node's overrides field.
-      if (!otherOverrideSet) {
-        return false
-      }
+      return false
+    }
+    if (!this.overrides) {
       this.overrides = otherOverrideSet
       this.recalculateOutEdgesOverrides()
       return true

From 6c9bbf9a51743a5897d0240431fd78438605fb35 Mon Sep 17 00:00:00 2001
From: Alon Navon <alon.n@sealsecurity.io>
Date: Mon, 28 Oct 2024 23:34:08 +0200
Subject: [PATCH 16/33] Parent doesn't matter to overrides

---
 workspaces/arborist/lib/node.js | 3 ---
 1 file changed, 3 deletions(-)

diff --git a/workspaces/arborist/lib/node.js b/workspaces/arborist/lib/node.js
index 5ec074e8766b1..3d20cd3f3367f 100644
--- a/workspaces/arborist/lib/node.js
+++ b/workspaces/arborist/lib/node.js
@@ -837,9 +837,6 @@ class Node {
       target.root = root
     }
 
-    if (!this.overrides && this.parent && this.parent.overrides) {
-      this.overrides = this.parent.overrides.getNodeRule(this)
-    }
     // tree should always be valid upon root setter completion.
     treeCheck(this)
     if (this !== root) {

From 63760d976159bf9311b5ef60ec07ded6d955eb02 Mon Sep 17 00:00:00 2001
From: Alon Navon <alon.n@sealsecurity.io>
Date: Mon, 28 Oct 2024 23:38:27 +0200
Subject: [PATCH 17/33] Update override set in reload

---
 workspaces/arborist/lib/edge.js | 18 +++++++++++++++++-
 1 file changed, 17 insertions(+), 1 deletion(-)

diff --git a/workspaces/arborist/lib/edge.js b/workspaces/arborist/lib/edge.js
index 2a8b520509b01..a0d9014b774e0 100644
--- a/workspaces/arborist/lib/edge.js
+++ b/workspaces/arborist/lib/edge.js
@@ -250,8 +250,20 @@ class Edge {
 
   reload (hard = false) {
     this.#explanation = null
+    let needToUpdateOverrideSet = false
+    let newOverrideSet
+    let oldOverrideSet
     if (this.#from.overrides) {
-      this.overrides = this.#from.overrides.getEdgeRule(this)
+      newOverrideSet = this.#from.overrides.getEdgeRule(this)
+      if (newOverrideSet && !newOverrideSet.isEqual(this.overrides)) {
+        needToUpdateOverrideSet = true
+        oldOverrideSet = this.overrides
+        this.overrides = newOverrideSet
+      }
+//      if (this.overrides !== this.#from.overrides) {
+//        console.log(this.#from.overrides)
+//        console.log(this.overrides)
+//      }
     } else {
       delete this.overrides
     }
@@ -268,6 +280,10 @@ class Edge {
     } else if (hard) {
       this.#error = null
     }
+    else if (needToUpdateOverrideSet) {
+      this.#to.updateNodeOverrideSetDueToEdgeRemoval(oldOverrideSet)
+      this.#to.updateNodeOverrideSet(newOverrideSet)
+    }
   }
 
   detach () {

From 61116300ba79fedb222d7793a9ce6f62ed32b33f Mon Sep 17 00:00:00 2001
From: Alon Navon <alon.n@sealsecurity.io>
Date: Mon, 28 Oct 2024 23:42:56 +0200
Subject: [PATCH 18/33] Erroneous node

---
 workspaces/arborist/lib/edge.js | 13 +++++++++----
 1 file changed, 9 insertions(+), 4 deletions(-)

diff --git a/workspaces/arborist/lib/edge.js b/workspaces/arborist/lib/edge.js
index a0d9014b774e0..86c884eaea8d3 100644
--- a/workspaces/arborist/lib/edge.js
+++ b/workspaces/arborist/lib/edge.js
@@ -179,6 +179,9 @@ class Edge {
 
   get spec () {
     if (this.overrides?.value && this.overrides.value !== '*' && this.overrides.name === this.#name) {
+      if (this.overrides === this.#from.overrides) {
+        return this.#spec
+      }
       if (this.overrides.value.startsWith('$')) {
         const ref = this.overrides.value.slice(1)
         // we may be a virtual root, if we are we want to resolve reference overrides
@@ -238,6 +241,12 @@ class Edge {
         this.#error = 'PEER LOCAL'
       } else if (!this.satisfiedBy(this.#to)) {
         this.#error = 'INVALID'
+      } else if (this.overrides && this.#to.edgesOut.size) {
+        if (!this.#to.findSpecificOverrideSet(this.overrides, this.#to.overrides)) {
+          // In principle any kind of difference here has some potential for problem,
+          // but we'll say it's invalid only if the override sets are plainly conflicting.
+          this.#error = 'INVALID'
+        }
       } else {
         this.#error = 'OK'
       }
@@ -260,10 +269,6 @@ class Edge {
         oldOverrideSet = this.overrides
         this.overrides = newOverrideSet
       }
-//      if (this.overrides !== this.#from.overrides) {
-//        console.log(this.#from.overrides)
-//        console.log(this.overrides)
-//      }
     } else {
       delete this.overrides
     }

From b42d4c8b9b8aff4fa0e898f9977887dcfd8e8715 Mon Sep 17 00:00:00 2001
From: Alon Navon <alon.n@sealsecurity.io>
Date: Tue, 29 Oct 2024 19:22:49 +0200
Subject: [PATCH 19/33] Add comments

---
 workspaces/arborist/lib/edge.js         | 21 +++++++++++++--------
 workspaces/arborist/lib/node.js         | 16 +++++++++++-----
 workspaces/arborist/lib/override-set.js |  1 +
 3 files changed, 25 insertions(+), 13 deletions(-)

diff --git a/workspaces/arborist/lib/edge.js b/workspaces/arborist/lib/edge.js
index 86c884eaea8d3..fd33e986990bd 100644
--- a/workspaces/arborist/lib/edge.js
+++ b/workspaces/arborist/lib/edge.js
@@ -179,9 +179,11 @@ class Edge {
 
   get spec () {
     if (this.overrides?.value && this.overrides.value !== '*' && this.overrides.name === this.#name) {
+      // If this edge has the same overrides field as the source, then we're not applying an override for this edge.
       if (this.overrides === this.#from.overrides) {
         return this.#spec
       }
+
       if (this.overrides.value.startsWith('$')) {
         const ref = this.overrides.value.slice(1)
         // we may be a virtual root, if we are we want to resolve reference overrides
@@ -241,12 +243,11 @@ class Edge {
         this.#error = 'PEER LOCAL'
       } else if (!this.satisfiedBy(this.#to)) {
         this.#error = 'INVALID'
-      } else if (this.overrides && this.#to.edgesOut.size) {
-        if (!this.#to.findSpecificOverrideSet(this.overrides, this.#to.overrides)) {
-          // In principle any kind of difference here has some potential for problem,
-          // but we'll say it's invalid only if the override sets are plainly conflicting.
-          this.#error = 'INVALID'
-        }
+      } else if (this.overrides && this.#to.edgesOut.size && !this.#to.findSpecificOverrideSet(this.overrides, this.#to.overrides)) {
+        // Any inconsistency between the edge's override set and the target's override set is potentially problematic.
+        // But we only say the edge is in error if the override sets are plainly conflicting.
+        // Note that if the target doesn't have any dependencies of their own, then this inconsistency is irrelevant.
+        this.#error = 'INVALID'
       } else {
         this.#error = 'OK'
       }
@@ -259,12 +260,15 @@ class Edge {
 
   reload (hard = false) {
     this.#explanation = null
+
     let needToUpdateOverrideSet = false
     let newOverrideSet
     let oldOverrideSet
     if (this.#from.overrides) {
       newOverrideSet = this.#from.overrides.getEdgeRule(this)
       if (newOverrideSet && !newOverrideSet.isEqual(this.overrides)) {
+        // If there's a new different override set we need to propagate it to the nodes.
+        // If we're deleting the override set then there's no point propagating it right now since it will be filled with another value later.
         needToUpdateOverrideSet = true
         oldOverrideSet = this.overrides
         this.overrides = newOverrideSet
@@ -286,8 +290,9 @@ class Edge {
       this.#error = null
     }
     else if (needToUpdateOverrideSet) {
-      this.#to.updateNodeOverrideSetDueToEdgeRemoval(oldOverrideSet)
-      this.#to.updateNodeOverrideSet(newOverrideSet)
+      // Propagate the new override set to the target node.
+      this.#to.updateOverridesEdgeInRemoved(oldOverrideSet)
+      this.#to.updateOverridesEdgeInAdded(newOverrideSet)
     }
   }
 
diff --git a/workspaces/arborist/lib/node.js b/workspaces/arborist/lib/node.js
index 3d20cd3f3367f..1f8887c32de18 100644
--- a/workspaces/arborist/lib/node.js
+++ b/workspaces/arborist/lib/node.js
@@ -351,6 +351,10 @@ class Node {
     if (this.overrides.name !== this.name) {
       return false
     }
+
+    // The overrides rule is for a package with this name, but some override rules only apply to specific
+    // versions. To make sure this package was actually overridden, we check whether any edge going in
+    // had the rule applied to it, in which case its overrides set is different than its source node.
     for (const edge of this.edgesIn) {
       if (this.overrides.isEqual(edge.overrides)) {
         if (!edge.overrides.isEqual(edge.from.overrides)) {
@@ -1018,6 +1022,8 @@ class Node {
       return false
     }
 
+    // If this node has no dependencies, then it's irrelevant to check the override
+    // rules of the replacement node.
     if (this.edgesOut.size) {
       // XXX need to check for two root nodes?
       if (node.overrides) {
@@ -1368,7 +1374,7 @@ class Node {
     for (const [, edge] of this.edgesOut) {
       edge.reload(true)
       if (edge.to) {
-        edge.to.updateNodeOverrideSet(edge.overrides)
+        edge.to.updateOverridesEdgeInAdded(edge.overrides)
       }
     }
   }
@@ -1387,7 +1393,7 @@ class Node {
     console.log('Conflicting override sets')
   }
 
-  updateNodeOverrideSetDueToEdgeRemoval (otherOverrideSet) {
+  updateOverridesEdgeInRemoved (otherOverrideSet) {
     // If this edge's overrides isn't equal to this node's overrides, then removing it won't change newOverrideSet later.
     if (!this.overrides || !this.overrides.isEqual(otherOverrideSet)) {
       return false
@@ -1420,7 +1426,7 @@ class Node {
   // The strictly correct logic is not to allow two edges with different overrides to point to the same node, because even if this node can satisfy
   // both, one of its dependencies might need to be different depending on the edge leading to it.
   // However, this might cause a lot of duplication, because the conflict in the dependencies might never actually happen.
-  updateNodeOverrideSet (otherOverrideSet) {
+  updateOverridesEdgeInAdded (otherOverrideSet) {
     if (!otherOverrideSet) {
       // Assuming there are any overrides at all, the overrides field is never undefined for any node at the end state of the tree.
       // So if the new edge's overrides is undefined it will be updated later. So we can wait with updating the node's overrides field.
@@ -1449,14 +1455,14 @@ class Node {
   deleteEdgeIn (edge) {
     this.edgesIn.delete(edge)
     if (edge.overrides) {
-      this.updateNodeOverrideSetDueToEdgeRemoval(edge.overrides)
+      this.updateOverridesEdgeInRemoved(edge.overrides)
     }
   }
 
   addEdgeIn (edge) {
     // We need to handle the case where the new edge in has an overrides field which is different from the current value.
     if (!this.overrides || !this.overrides.isEqual(edge.overrides)) {
-      this.updateNodeOverrideSet(edge.overrides)
+      this.updateOverridesEdgeInAdded(edge.overrides)
     }
 
     this.edgesIn.add(edge)
diff --git a/workspaces/arborist/lib/override-set.js b/workspaces/arborist/lib/override-set.js
index 8a64333204f9a..48a43ebbcf6ac 100644
--- a/workspaces/arborist/lib/override-set.js
+++ b/workspaces/arborist/lib/override-set.js
@@ -92,6 +92,7 @@ class OverrideSet {
         return rule
       }
 
+      // We need to use the rawSpec here, because the spec has the overrides applied to it already.
       let spec = npa(`${edge.name}@${edge.rawSpec}`)
       if (spec.type === 'alias') {
         spec = spec.subSpec

From 7d977263e14949f41febc5662c6235e10570f34c Mon Sep 17 00:00:00 2001
From: Alon Navon <alon.n@sealsecurity.io>
Date: Mon, 4 Nov 2024 16:28:31 +0200
Subject: [PATCH 20/33] Code review fixes

---
 workspaces/arborist/lib/node.js | 5 ++++-
 1 file changed, 4 insertions(+), 1 deletion(-)

diff --git a/workspaces/arborist/lib/node.js b/workspaces/arborist/lib/node.js
index 1f8887c32de18..d028a22790e0a 100644
--- a/workspaces/arborist/lib/node.js
+++ b/workspaces/arborist/lib/node.js
@@ -65,6 +65,8 @@ const CaseInsensitiveMap = require('./case-insensitive-map.js')
 
 const querySelectorAll = require('./query-selector-all.js')
 
+const log = require('proc-log')
+
 class Node {
   #global
   #meta
@@ -1390,7 +1392,7 @@ class Node {
         return first
       }
     }
-    console.log('Conflicting override sets')
+    log.silly('Conflicting override sets', this, first, second)
   }
 
   updateOverridesEdgeInRemoved (otherOverrideSet) {
@@ -1450,6 +1452,7 @@ class Node {
       return false
     }
     // This is an error condition. We can only get here if the new override set is in conflict with the existing.
+    throw Object.assign(new Error(`Conflicting override requirements for node ${this.name}`), { code: 'EOVERRIDE' })
   }
 
   deleteEdgeIn (edge) {

From 35e5790bc6ef0f7bec3aa325f86554f73d7e4b26 Mon Sep 17 00:00:00 2001
From: Alon Navon <alon.n@sealsecurity.io>
Date: Mon, 4 Nov 2024 16:32:54 +0200
Subject: [PATCH 21/33] Fix edge satisfaction logic

---
 workspaces/arborist/lib/dep-valid.js |  2 +-
 workspaces/arborist/lib/edge.js      | 26 +++++++++++++++++++++++++-
 workspaces/arborist/lib/node.js      |  2 +-
 3 files changed, 27 insertions(+), 3 deletions(-)

diff --git a/workspaces/arborist/lib/dep-valid.js b/workspaces/arborist/lib/dep-valid.js
index 4afb5e47cf111..4fab2d3ece835 100644
--- a/workspaces/arborist/lib/dep-valid.js
+++ b/workspaces/arborist/lib/dep-valid.js
@@ -101,7 +101,7 @@ const depValid = (child, requested, requestor) => {
       })
     }
 
-    default: // unpossible, just being cautious
+    default: // impossible, just being cautious
       break
   }
 
diff --git a/workspaces/arborist/lib/edge.js b/workspaces/arborist/lib/edge.js
index fd33e986990bd..83c3c5d6eaf7f 100644
--- a/workspaces/arborist/lib/edge.js
+++ b/workspaces/arborist/lib/edge.js
@@ -112,7 +112,31 @@ class Edge {
     if (node.hasShrinkwrap || node.inShrinkwrap || node.inBundle) {
       return depValid(node, this.rawSpec, this.#accept, this.#from)
     }
-    return depValid(node, this.spec, this.#accept, this.#from)
+
+    // If there's no override we just use the spec.
+    if (!this.overridden) {
+      return depValid(node, this.spec, this.#accept, this.#from)
+    }
+    // There's some override. If the target node satisfies the overriding spec
+    // then it's okay.
+    if (depValid(node, this.spec, this.#accept, this.#from)) {
+      return true
+    }
+    // If it doesn't, then it should at least satisfy the original spec.
+    if (!depValid(node, this.rawSpec, this.#accept, this.#from)) {
+      return false
+    }
+    // It satisfies the original spec, not the overriding spec. We need to make
+    // sure it doesn't use the overridden spec.
+    // For example:
+    //   we might have an ^8.0.0 rawSpec, and an override that makes
+    //   keySpec=8.23.0 and the override value spec=9.0.0.
+    //   If the node is 9.0.0, then it's okay because it's consistent with spec.
+    //   If the node is 8.24.0, then it's okay because it's consistent with the rawSpec.
+    //   If the node is 8.23.0, then it's not okay because even though it's consistent
+    //   with the rawSpec, it's also consistent with the keySpec.
+    //   So we're looking for ^8.0.0 or 9.0.0 and not 8.23.0.
+    return !depValid(node, this.overrides.keySpec, this.#accept, this.#from)
   }
 
   // return the edge data, and an explanation of how that edge came to be here
diff --git a/workspaces/arborist/lib/node.js b/workspaces/arborist/lib/node.js
index d028a22790e0a..f2c0459769ba5 100644
--- a/workspaces/arborist/lib/node.js
+++ b/workspaces/arborist/lib/node.js
@@ -358,7 +358,7 @@ class Node {
     // versions. To make sure this package was actually overridden, we check whether any edge going in
     // had the rule applied to it, in which case its overrides set is different than its source node.
     for (const edge of this.edgesIn) {
-      if (this.overrides.isEqual(edge.overrides)) {
+      if (edge.overrides && edge.overrides.name === this.name && edge.overrides.value === this.version) {
         if (!edge.overrides.isEqual(edge.from.overrides)) {
           return true
         }

From 35ce2f743aaa68dda908467843153f46e209cf0a Mon Sep 17 00:00:00 2001
From: Alon Navon <alon.n@sealsecurity.io>
Date: Mon, 4 Nov 2024 16:34:28 +0200
Subject: [PATCH 22/33] Fix dedupe logic

---
 workspaces/arborist/lib/node.js | 9 +++++++--
 1 file changed, 7 insertions(+), 2 deletions(-)

diff --git a/workspaces/arborist/lib/node.js b/workspaces/arborist/lib/node.js
index f2c0459769ba5..5d346a246c1cf 100644
--- a/workspaces/arborist/lib/node.js
+++ b/workspaces/arborist/lib/node.js
@@ -1106,8 +1106,13 @@ class Node {
       return false
     }
 
-    // if we prefer dedupe, or if the version is greater/equal, take the other
-    if (preferDedupe || semver.gte(other.version, this.version)) {
+    // if we prefer dedupe, or if the version is equal, take the other
+    if (preferDedupe || semver.eq(other.version, this.version)) {
+      return true
+    }
+
+    // if our current version isn't the result of an override, then prefer to take the greater version
+    if (!this.overridden && semver.gt(other.version, this.version)) {
       return true
     }
 

From 56309540cd0a153666ca16fec0d550e1d7853c03 Mon Sep 17 00:00:00 2001
From: Alon Navon <alon.n@sealsecurity.io>
Date: Mon, 4 Nov 2024 18:46:17 +0200
Subject: [PATCH 23/33] Code review fixes

---
 workspaces/arborist/lib/edge.js | 12 ++++++++----
 workspaces/arborist/lib/node.js |  4 ++--
 2 files changed, 10 insertions(+), 6 deletions(-)

diff --git a/workspaces/arborist/lib/edge.js b/workspaces/arborist/lib/edge.js
index 83c3c5d6eaf7f..d547fda715dad 100644
--- a/workspaces/arborist/lib/edge.js
+++ b/workspaces/arborist/lib/edge.js
@@ -204,7 +204,7 @@ class Edge {
   get spec () {
     if (this.overrides?.value && this.overrides.value !== '*' && this.overrides.name === this.#name) {
       // If this edge has the same overrides field as the source, then we're not applying an override for this edge.
-      if (this.overrides === this.#from.overrides) {
+      if (this.overrides === this.#from?.overrides) {
         return this.#spec
       }
 
@@ -212,9 +212,9 @@ class Edge {
         const ref = this.overrides.value.slice(1)
         // we may be a virtual root, if we are we want to resolve reference overrides
         // from the real root, not the virtual one
-        const pkg = this.#from.sourceReference
-          ? this.#from.sourceReference.root.package
-          : this.#from.root.package
+        const pkg = this.#from?.sourceReference
+          ? this.#from?.sourceReference.root.package
+          : this.#from?.root?.package
         if (pkg.devDependencies?.[ref]) {
           return pkg.devDependencies[ref]
         }
@@ -264,6 +264,7 @@ class Edge {
           this.#error = 'MISSING'
         }
       } else if (this.peer && this.#from === this.#to.parent && !this.#from.isTop) {
+      } else if (this.peer && this.#from === this.#to.parent && !this.#from?.isTop) {
         this.#error = 'PEER LOCAL'
       } else if (!this.satisfiedBy(this.#to)) {
         this.#error = 'INVALID'
@@ -289,6 +290,7 @@ class Edge {
     let newOverrideSet
     let oldOverrideSet
     if (this.#from.overrides) {
+    if (this.#from?.overrides) {
       newOverrideSet = this.#from.overrides.getEdgeRule(this)
       if (newOverrideSet && !newOverrideSet.isEqual(this.overrides)) {
         // If there's a new different override set we need to propagate it to the nodes.
@@ -301,6 +303,7 @@ class Edge {
       delete this.overrides
     }
     const newTo = this.#from.resolve(this.#name)
+    const newTo = this.#from?.resolve(this.#name)
     if (newTo !== this.#to) {
       if (this.#to) {
         this.#to.deleteEdgeIn(this)
@@ -326,6 +329,7 @@ class Edge {
       this.#to.deleteEdgeIn(this)
     }
     this.#from.edgesOut.delete(this.#name)
+    this.#from?.edgesOut.delete(this.#name)
     this.#to = null
     this.#error = 'DETACHED'
     this.#from = null
diff --git a/workspaces/arborist/lib/node.js b/workspaces/arborist/lib/node.js
index 5d346a246c1cf..7b2c837cfaacd 100644
--- a/workspaces/arborist/lib/node.js
+++ b/workspaces/arborist/lib/node.js
@@ -1378,7 +1378,7 @@ class Node {
 
   recalculateOutEdgesOverrides () {
     // For each edge out propogate the new overrides through.
-    for (const [, edge] of this.edgesOut) {
+    for (const edge of this.edgesOut.values()) {
       edge.reload(true)
       if (edge.to) {
         edge.to.updateOverridesEdgeInAdded(edge.overrides)
@@ -1386,7 +1386,7 @@ class Node {
     }
   }
 
-  findSpecificOverrideSet (first, second) {
+  static findSpecificOverrideSet (first, second) {
     for (let overrideSet = second; overrideSet; overrideSet = overrideSet.parent) {
       if (overrideSet.isEqual(first)) {
         return second

From f859e926f26f521bd8fd089a135aa1bfc6600b41 Mon Sep 17 00:00:00 2001
From: Alon Navon <alon.n@sealsecurity.io>
Date: Mon, 4 Nov 2024 19:43:48 +0200
Subject: [PATCH 24/33] Oops

---
 workspaces/arborist/lib/edge.js | 4 ----
 1 file changed, 4 deletions(-)

diff --git a/workspaces/arborist/lib/edge.js b/workspaces/arborist/lib/edge.js
index d547fda715dad..8fab290e5c174 100644
--- a/workspaces/arborist/lib/edge.js
+++ b/workspaces/arborist/lib/edge.js
@@ -263,7 +263,6 @@ class Edge {
         } else {
           this.#error = 'MISSING'
         }
-      } else if (this.peer && this.#from === this.#to.parent && !this.#from.isTop) {
       } else if (this.peer && this.#from === this.#to.parent && !this.#from?.isTop) {
         this.#error = 'PEER LOCAL'
       } else if (!this.satisfiedBy(this.#to)) {
@@ -289,7 +288,6 @@ class Edge {
     let needToUpdateOverrideSet = false
     let newOverrideSet
     let oldOverrideSet
-    if (this.#from.overrides) {
     if (this.#from?.overrides) {
       newOverrideSet = this.#from.overrides.getEdgeRule(this)
       if (newOverrideSet && !newOverrideSet.isEqual(this.overrides)) {
@@ -302,7 +300,6 @@ class Edge {
     } else {
       delete this.overrides
     }
-    const newTo = this.#from.resolve(this.#name)
     const newTo = this.#from?.resolve(this.#name)
     if (newTo !== this.#to) {
       if (this.#to) {
@@ -328,7 +325,6 @@ class Edge {
     if (this.#to) {
       this.#to.deleteEdgeIn(this)
     }
-    this.#from.edgesOut.delete(this.#name)
     this.#from?.edgesOut.delete(this.#name)
     this.#to = null
     this.#error = 'DETACHED'

From 29f07506bc4c19ea16705d8a76ecee0a082129e1 Mon Sep 17 00:00:00 2001
From: Alon Navon <alon.n@sealsecurity.io>
Date: Tue, 5 Nov 2024 14:54:54 +0200
Subject: [PATCH 25/33] Static function

---
 workspaces/arborist/lib/edge.js | 2 +-
 workspaces/arborist/lib/node.js | 4 ++--
 2 files changed, 3 insertions(+), 3 deletions(-)

diff --git a/workspaces/arborist/lib/edge.js b/workspaces/arborist/lib/edge.js
index 8fab290e5c174..9b404d2028e5d 100644
--- a/workspaces/arborist/lib/edge.js
+++ b/workspaces/arborist/lib/edge.js
@@ -267,7 +267,7 @@ class Edge {
         this.#error = 'PEER LOCAL'
       } else if (!this.satisfiedBy(this.#to)) {
         this.#error = 'INVALID'
-      } else if (this.overrides && this.#to.edgesOut.size && !this.#to.findSpecificOverrideSet(this.overrides, this.#to.overrides)) {
+      } else if (this.overrides && this.#to.edgesOut.size && !this.#to.constructor.findSpecificOverrideSet(this.overrides, this.#to.overrides)) {
         // Any inconsistency between the edge's override set and the target's override set is potentially problematic.
         // But we only say the edge is in error if the override sets are plainly conflicting.
         // Note that if the target doesn't have any dependencies of their own, then this inconsistency is irrelevant.
diff --git a/workspaces/arborist/lib/node.js b/workspaces/arborist/lib/node.js
index 7b2c837cfaacd..a7a6304a4ce34 100644
--- a/workspaces/arborist/lib/node.js
+++ b/workspaces/arborist/lib/node.js
@@ -1408,7 +1408,7 @@ class Node {
     let newOverrideSet
     for (const edge of this.edgesIn) {
       if (newOverrideSet && edge.overrides) {
-        newOverrideSet = this.findSpecificOverrideSet(edge.overrides, newOverrideSet)
+        newOverrideSet = Node.findSpecificOverrideSet(edge.overrides, newOverrideSet)
       } else {
         newOverrideSet = edge.overrides
       }
@@ -1447,7 +1447,7 @@ class Node {
     if (this.overrides.isEqual(otherOverrideSet)) {
       return false
     }
-    const newOverrideSet = this.findSpecificOverrideSet(this.overrides, otherOverrideSet)
+    const newOverrideSet = Node.findSpecificOverrideSet(this.overrides, otherOverrideSet)
     if (newOverrideSet) {
       if (!this.overrides.isEqual(newOverrideSet)) {
         this.overrides = newOverrideSet

From 17ec4571f1ba9e8db3b4f9fce1115e3c2568d105 Mon Sep 17 00:00:00 2001
From: Alon Navon <alon.n@sealsecurity.io>
Date: Tue, 5 Nov 2024 19:26:38 +0200
Subject: [PATCH 26/33] Typo

---
 workspaces/arborist/lib/edge.js | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/workspaces/arborist/lib/edge.js b/workspaces/arborist/lib/edge.js
index 9b404d2028e5d..2ab3dd9227187 100644
--- a/workspaces/arborist/lib/edge.js
+++ b/workspaces/arborist/lib/edge.js
@@ -114,7 +114,7 @@ class Edge {
     }
 
     // If there's no override we just use the spec.
-    if (!this.overridden) {
+    if (!this.overrides) {
       return depValid(node, this.spec, this.#accept, this.#from)
     }
     // There's some override. If the target node satisfies the overriding spec

From 4cde831b7f2be489bf9f8576b977a4719774bc37 Mon Sep 17 00:00:00 2001
From: Alon Navon <alon.n@sealsecurity.io>
Date: Thu, 7 Nov 2024 20:13:27 +0200
Subject: [PATCH 27/33] Shouldn't throw

---
 workspaces/arborist/lib/edge.js | 2 +-
 workspaces/arborist/lib/node.js | 6 +++++-
 2 files changed, 6 insertions(+), 2 deletions(-)

diff --git a/workspaces/arborist/lib/edge.js b/workspaces/arborist/lib/edge.js
index 2ab3dd9227187..55468bba15b3c 100644
--- a/workspaces/arborist/lib/edge.js
+++ b/workspaces/arborist/lib/edge.js
@@ -267,7 +267,7 @@ class Edge {
         this.#error = 'PEER LOCAL'
       } else if (!this.satisfiedBy(this.#to)) {
         this.#error = 'INVALID'
-      } else if (this.overrides && this.#to.edgesOut.size && !this.#to.constructor.findSpecificOverrideSet(this.overrides, this.#to.overrides)) {
+      } else if (this.overrides && this.#to.edgesOut.size && this.#to.constructor.doOverrideSetsConflict(this.overrides, this.#to.overrides)) {
         // Any inconsistency between the edge's override set and the target's override set is potentially problematic.
         // But we only say the edge is in error if the override sets are plainly conflicting.
         // Note that if the target doesn't have any dependencies of their own, then this inconsistency is irrelevant.
diff --git a/workspaces/arborist/lib/node.js b/workspaces/arborist/lib/node.js
index a7a6304a4ce34..836cc0dd6c69f 100644
--- a/workspaces/arborist/lib/node.js
+++ b/workspaces/arborist/lib/node.js
@@ -1400,6 +1400,10 @@ class Node {
     log.silly('Conflicting override sets', this, first, second)
   }
 
+  static doOverrideSetsConflict (first, second) {
+    return (this.findSpecificOverrideSet(first, second) === undefined)
+  }
+
   updateOverridesEdgeInRemoved (otherOverrideSet) {
     // If this edge's overrides isn't equal to this node's overrides, then removing it won't change newOverrideSet later.
     if (!this.overrides || !this.overrides.isEqual(otherOverrideSet)) {
@@ -1457,7 +1461,7 @@ class Node {
       return false
     }
     // This is an error condition. We can only get here if the new override set is in conflict with the existing.
-    throw Object.assign(new Error(`Conflicting override requirements for node ${this.name}`), { code: 'EOVERRIDE' })
+    log.silly('Conflicting override sets', this.name)
   }
 
   deleteEdgeIn (edge) {

From 701b125b9ae3b02d7132646fe5ccede86ddabe52 Mon Sep 17 00:00:00 2001
From: Alon Navon <alon.n@sealsecurity.io>
Date: Thu, 7 Nov 2024 20:23:47 +0200
Subject: [PATCH 28/33] CR fixes

---
 workspaces/arborist/lib/edge.js | 2 ++
 workspaces/arborist/lib/node.js | 2 +-
 2 files changed, 3 insertions(+), 1 deletion(-)

diff --git a/workspaces/arborist/lib/edge.js b/workspaces/arborist/lib/edge.js
index 55468bba15b3c..b27100e42c15e 100644
--- a/workspaces/arborist/lib/edge.js
+++ b/workspaces/arborist/lib/edge.js
@@ -104,6 +104,7 @@ class Edge {
 
   satisfiedBy (node) {
     if (node.name !== this.#name) {
+    if (node.name !== this.#name || !this.#from) {
       return false
     }
 
@@ -115,6 +116,7 @@ class Edge {
 
     // If there's no override we just use the spec.
     if (!this.overrides) {
+    if (!this.overrides?.keySpec) {
       return depValid(node, this.spec, this.#accept, this.#from)
     }
     // There's some override. If the target node satisfies the overriding spec
diff --git a/workspaces/arborist/lib/node.js b/workspaces/arborist/lib/node.js
index 836cc0dd6c69f..0779d9eab6b72 100644
--- a/workspaces/arborist/lib/node.js
+++ b/workspaces/arborist/lib/node.js
@@ -1397,7 +1397,7 @@ class Node {
         return first
       }
     }
-    log.silly('Conflicting override sets', this, first, second)
+    log.silly('Conflicting override sets', first, second)
   }
 
   static doOverrideSetsConflict (first, second) {

From b119f67abc640edcafaeffbed7b2caf99d2660f6 Mon Sep 17 00:00:00 2001
From: Alon Navon <alon.n@sealsecurity.io>
Date: Thu, 7 Nov 2024 20:35:08 +0200
Subject: [PATCH 29/33] CR fixes

---
 workspaces/arborist/lib/edge.js | 2 --
 1 file changed, 2 deletions(-)

diff --git a/workspaces/arborist/lib/edge.js b/workspaces/arborist/lib/edge.js
index b27100e42c15e..1f42c2e8ea209 100644
--- a/workspaces/arborist/lib/edge.js
+++ b/workspaces/arborist/lib/edge.js
@@ -103,7 +103,6 @@ class Edge {
   }
 
   satisfiedBy (node) {
-    if (node.name !== this.#name) {
     if (node.name !== this.#name || !this.#from) {
       return false
     }
@@ -115,7 +114,6 @@ class Edge {
     }
 
     // If there's no override we just use the spec.
-    if (!this.overrides) {
     if (!this.overrides?.keySpec) {
       return depValid(node, this.spec, this.#accept, this.#from)
     }

From 3136b644c27d62060ea0741129ec6de7cf6fdf9a Mon Sep 17 00:00:00 2001
From: Alon Navon <alon.n@sealsecurity.io>
Date: Sun, 10 Nov 2024 16:39:08 +0200
Subject: [PATCH 30/33] Move functions

---
 workspaces/arborist/lib/edge.js         |  3 ++-
 workspaces/arborist/lib/node.js         | 22 ++--------------------
 workspaces/arborist/lib/override-set.js | 19 +++++++++++++++++++
 3 files changed, 23 insertions(+), 21 deletions(-)

diff --git a/workspaces/arborist/lib/edge.js b/workspaces/arborist/lib/edge.js
index 1f42c2e8ea209..77f351ed45789 100644
--- a/workspaces/arborist/lib/edge.js
+++ b/workspaces/arborist/lib/edge.js
@@ -4,6 +4,7 @@
 const util = require('util')
 const npa = require('npm-package-arg')
 const depValid = require('./dep-valid.js')
+const OverrideSet = require('./override-set.js')
 
 class ArboristEdge {
   constructor (edge) {
@@ -267,7 +268,7 @@ class Edge {
         this.#error = 'PEER LOCAL'
       } else if (!this.satisfiedBy(this.#to)) {
         this.#error = 'INVALID'
-      } else if (this.overrides && this.#to.edgesOut.size && this.#to.constructor.doOverrideSetsConflict(this.overrides, this.#to.overrides)) {
+      } else if (this.overrides && this.#to.edgesOut.size && OverrideSet.doOverrideSetsConflict(this.overrides, this.#to.overrides)) {
         // Any inconsistency between the edge's override set and the target's override set is potentially problematic.
         // But we only say the edge is in error if the override sets are plainly conflicting.
         // Note that if the target doesn't have any dependencies of their own, then this inconsistency is irrelevant.
diff --git a/workspaces/arborist/lib/node.js b/workspaces/arborist/lib/node.js
index 0779d9eab6b72..0640c3c0f441d 100644
--- a/workspaces/arborist/lib/node.js
+++ b/workspaces/arborist/lib/node.js
@@ -1386,24 +1386,6 @@ class Node {
     }
   }
 
-  static findSpecificOverrideSet (first, second) {
-    for (let overrideSet = second; overrideSet; overrideSet = overrideSet.parent) {
-      if (overrideSet.isEqual(first)) {
-        return second
-      }
-    }
-    for (let overrideSet = first; overrideSet; overrideSet = overrideSet.parent) {
-      if (overrideSet.isEqual(second)) {
-        return first
-      }
-    }
-    log.silly('Conflicting override sets', first, second)
-  }
-
-  static doOverrideSetsConflict (first, second) {
-    return (this.findSpecificOverrideSet(first, second) === undefined)
-  }
-
   updateOverridesEdgeInRemoved (otherOverrideSet) {
     // If this edge's overrides isn't equal to this node's overrides, then removing it won't change newOverrideSet later.
     if (!this.overrides || !this.overrides.isEqual(otherOverrideSet)) {
@@ -1412,7 +1394,7 @@ class Node {
     let newOverrideSet
     for (const edge of this.edgesIn) {
       if (newOverrideSet && edge.overrides) {
-        newOverrideSet = Node.findSpecificOverrideSet(edge.overrides, newOverrideSet)
+        newOverrideSet = OverrideSet.findSpecificOverrideSet(edge.overrides, newOverrideSet)
       } else {
         newOverrideSet = edge.overrides
       }
@@ -1451,7 +1433,7 @@ class Node {
     if (this.overrides.isEqual(otherOverrideSet)) {
       return false
     }
-    const newOverrideSet = Node.findSpecificOverrideSet(this.overrides, otherOverrideSet)
+    const newOverrideSet = OverrideSet.findSpecificOverrideSet(this.overrides, otherOverrideSet)
     if (newOverrideSet) {
       if (!this.overrides.isEqual(newOverrideSet)) {
         this.overrides = newOverrideSet
diff --git a/workspaces/arborist/lib/override-set.js b/workspaces/arborist/lib/override-set.js
index 48a43ebbcf6ac..9c8dd6ea72a40 100644
--- a/workspaces/arborist/lib/override-set.js
+++ b/workspaces/arborist/lib/override-set.js
@@ -1,5 +1,6 @@
 const npa = require('npm-package-arg')
 const semver = require('semver')
+const log = require('proc-log')
 
 class OverrideSet {
   constructor ({ overrides, key, parent }) {
@@ -180,6 +181,24 @@ class OverrideSet {
 
     return ruleset
   }
+
+  static findSpecificOverrideSet (first, second) {
+    for (let overrideSet = second; overrideSet; overrideSet = overrideSet.parent) {
+      if (overrideSet.isEqual(first)) {
+        return second
+      }
+    }
+    for (let overrideSet = first; overrideSet; overrideSet = overrideSet.parent) {
+      if (overrideSet.isEqual(second)) {
+        return first
+      }
+    }
+    log.silly('Conflicting override sets', first, second)
+  }
+  
+  static doOverrideSetsConflict (first, second) {
+    return (this.findSpecificOverrideSet(first, second) === undefined)
+  }
 }
 
 module.exports = OverrideSet

From 80126c764f377d9136c79dd5b45b65e718ea91e8 Mon Sep 17 00:00:00 2001
From: Alon Navon <alon.n@sealsecurity.io>
Date: Mon, 11 Nov 2024 17:47:55 +0200
Subject: [PATCH 31/33] Comments

---
 workspaces/arborist/lib/override-set.js | 5 +++++
 1 file changed, 5 insertions(+)

diff --git a/workspaces/arborist/lib/override-set.js b/workspaces/arborist/lib/override-set.js
index 9c8dd6ea72a40..82607e32c6ddc 100644
--- a/workspaces/arborist/lib/override-set.js
+++ b/workspaces/arborist/lib/override-set.js
@@ -193,10 +193,15 @@ class OverrideSet {
         return first
       }
     }
+
+    // The override sets are incomparable. Neither one contains the other.
     log.silly('Conflicting override sets', first, second)
   }
   
   static doOverrideSetsConflict (first, second) {
+    // If override sets contain one another then we can try to use the more specific one.
+    // This is imperfect, since we may lose some of the context of the less specific override
+    // set. But the more specific override set is more important to propagate, so we go with it.
     return (this.findSpecificOverrideSet(first, second) === undefined)
   }
 }

From c4b5338f44362a7400e0612ca164e4572ea989a6 Mon Sep 17 00:00:00 2001
From: Alon Navon <alon.n@sealsecurity.io>
Date: Mon, 11 Nov 2024 19:24:28 +0200
Subject: [PATCH 32/33] Override sets unit tests

---
 workspaces/arborist/test/override-set.js | 117 +++++++++++++++++++++++
 1 file changed, 117 insertions(+)

diff --git a/workspaces/arborist/test/override-set.js b/workspaces/arborist/test/override-set.js
index 705996b443b22..d34535c1dbbaa 100644
--- a/workspaces/arborist/test/override-set.js
+++ b/workspaces/arborist/test/override-set.js
@@ -271,4 +271,121 @@ t.test('constructor', async (t) => {
     const outOfRangeRule = bazEdgeRule.getEdgeRule({ name: 'buzz', spec: 'github:baz/buzz#semver:^2.0.0' })
     t.equal(outOfRangeRule.name, 'baz', 'no match - returned parent')
   })
+
+  t.test('isequal and findspecificoverrideset tests', async (t) => {
+    const overrides1 = new OverrideSet({
+      overrides: {
+        foo: {
+          bar: {
+            '.': '2.0.0',
+            baz: '3.0.0',
+          },
+          baz: '2.0.0',
+        },
+        bar: '1.0.0',
+        baz: '1.0.0',
+      },
+    })
+    const overrides2 = new OverrideSet({
+      overrides: {
+        foo: {
+          bar: {
+            '.': '2.0.0',
+            baz: '3.0.0',
+          },
+          baz: '2.0.0',
+        },
+        bar: '1.0.0',
+        baz: '1.0.0',
+      },
+    })
+    const overrides3 = new OverrideSet({
+      overrides: {
+        foo: {
+          bar: {
+            '.': '2.0.0',
+            baz: '3.1.0',
+          },
+          baz: '2.0.0',
+        },
+        bar: '1.0.0',
+        baz: '1.0.0',
+      },
+    })
+    const overrides4 = new OverrideSet({
+      overrides: {
+        foo: {
+          bar: {
+            '.': '2.0.0',
+          },
+          baz: '2.0.0',
+        },
+        bar: '1.0.0',
+        baz: '1.0.0',
+      },
+    })
+    const overrides5 = new OverrideSet({
+      overrides: {
+        foo: {
+          bar: {
+            '.': '2.0.0',
+          },
+          bat: '2.0.0',
+        },
+        bar: '1.0.0',
+        baz: '1.0.0',
+      },
+    })
+    const overrides6 = new OverrideSet({
+      overrides: {
+        bar: {
+          '.': '2.0.0',
+        },
+        bat: '2.0.0',
+      },
+    })
+    const overrides7 = new OverrideSet({
+      overrides: {
+        bat: '2.0.0',
+      },
+    })
+    const overrides8 = new OverrideSet({
+      overrides: {
+        bat: '1.2.0',
+      },
+    })
+    const overrides9 = new OverrideSet({
+      overrides: {
+        'bat@3.0.0': '1.2.0',
+      },
+    })
+
+    t.ok(overrides1.isEqual(overrides1), 'override set is equal to itself')
+    t.ok(overrides1.isEqual(overrides2), 'two identical override sets are equal')
+    t.ok(!overrides1.isEqual(overrides3), 'two different override sets are not equal')
+    t.ok(!overrides2.isEqual(overrides3), 'two different override sets are not equal')
+    t.ok(!overrides3.isEqual(overrides1), 'two different override sets are not equal')
+    t.ok(!overrides3.isEqual(overrides2), 'two different override sets are not equal')
+    t.ok(!overrides4.isEqual(overrides1), 'two different override sets are not equal')
+    t.ok(!overrides4.isEqual(overrides2), 'two different override sets are not equal')
+    t.ok(!overrides4.isEqual(overrides3), 'two different override sets are not equal')
+    t.ok(!overrides4.isEqual(overrides5), 'two override sets that differ only by package name are not equal')
+    t.ok(!overrides5.isEqual(overrides4), 'two override sets that differ only by package name are not equal')
+    t.equal(OverrideSet.findSpecificOverrideSet(overrides5, overrides5), overrides5, "find more specific override set when the sets are identical")
+    t.equal(OverrideSet.findSpecificOverrideSet(overrides5, overrides6), overrides6, "find more specific override set when it's the second")
+    t.equal(OverrideSet.findSpecificOverrideSet(overrides6, overrides5), overrides6, "find more specific override set when it's the first")
+    t.ok(!OverrideSet.doOverrideSetsConflict(overrides1, overrides2), "override sets are equal")
+    t.ok(!OverrideSet.doOverrideSetsConflict(overrides5, overrides5), "override sets are the same object")
+    t.ok(!OverrideSet.doOverrideSetsConflict(overrides5, overrides6), "one override set is the specific version of the other")
+    t.ok(!OverrideSet.doOverrideSetsConflict(overrides6, overrides5), "one override set is the specific version of the other")
+    t.ok(OverrideSet.doOverrideSetsConflict(overrides5, overrides7), "no override set is the specific version of the other")
+    t.ok(OverrideSet.doOverrideSetsConflict(overrides7, overrides5), "no override set is the specific version of the other")
+    t.ok(!overrides7.isEqual(overrides8), 'two override sets that differ in the version are not equal')
+    t.ok(!overrides8.isEqual(overrides9), 'two override sets that differ in the range are not equal')
+    t.ok(!overrides7.isEqual(overrides9), 'two override sets that differ in both version and range are not equal')
+    t.ok(OverrideSet.doOverrideSetsConflict(overrides7, overrides8), "override sets are incomparable due to version")
+    t.ok(OverrideSet.doOverrideSetsConflict(overrides7, overrides9), "override sets are incomparable due to version and range")
+    t.ok(OverrideSet.doOverrideSetsConflict(overrides8, overrides9), "override sets are incomparable due to range")
+
+  })
 })

From 9db123265e34037a2175780868670a7c0f68c619 Mon Sep 17 00:00:00 2001
From: Alon Navon <alon.n@sealsecurity.io>
Date: Mon, 11 Nov 2024 19:48:13 +0200
Subject: [PATCH 33/33] Clarify comment

---
 workspaces/arborist/lib/override-set.js | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

diff --git a/workspaces/arborist/lib/override-set.js b/workspaces/arborist/lib/override-set.js
index 82607e32c6ddc..13c5982b44414 100644
--- a/workspaces/arborist/lib/override-set.js
+++ b/workspaces/arborist/lib/override-set.js
@@ -200,8 +200,7 @@ class OverrideSet {
   
   static doOverrideSetsConflict (first, second) {
     // If override sets contain one another then we can try to use the more specific one.
-    // This is imperfect, since we may lose some of the context of the less specific override
-    // set. But the more specific override set is more important to propagate, so we go with it.
+    // If neither one is more specific, then we consider them to be in conflict.
     return (this.findSpecificOverrideSet(first, second) === undefined)
   }
 }