fix(query): report logical dep location under linked strategy

lib/commands/query.js

@@ -2,15 +2,27 @@ const { resolve } = require('node:path')
 const BaseCommand = require('../base-cmd.js')
 const { log, output } = require('proc-log')
 
+// Ranks competing representations of the same physical package so the most logical one is reported.
+// A top-level placement (e.g. node_modules/<pkg>) beats the canonical store node, which beats an internal store symlink.
+const locationRank = (node) => {
+  if (!node.location.includes('node_modules/.store/')) {
+    return 2
+  }
+  return node.isLink ? 0 : 1
+}
+
 class QuerySelectorItem {
   constructor (node) {
     // all enumerable properties from the target
     Object.assign(this, node.target.package)
 
     // append extra info
     this.pkgid = node.target.pkgid
-    this.location = node.target.location
-    this.path = node.target.path
+    // For a dep symlinked into the isolated store, report the logical link location (node_modules/<pkg>) rather than the .store backing path.
+    // Workspaces and regular nodes keep the target location (e.g. packages/<ws>).
+    const logical = node.target.isInStore ? node : node.target
+    this.location = logical.location
+    this.path = logical.path
     this.realpath = node.target.realpath
     this.resolved = node.target.resolved
     this.from = []
@@ -33,7 +45,7 @@ class QuerySelectorItem {
 
 class Query extends BaseCommand {
   #response = [] // response is the query response
-  #seen = new Set() // paths we've seen so we can keep response deduped
+  #seen = new Map() // physical location -> index in #response, to keep response deduped
 
   static description = 'Retrieve a filtered list of packages'
   static name = 'query'
@@ -127,12 +139,22 @@ class Query extends BaseCommand {
   async #queryTree (tree, arg) {
     const items = await tree.querySelectorAll(arg, this.npm.flatOptions)
     for (const node of items) {
+      // Dedup by the target's physical location so multiple logical links to the same store node collapse to one result.
       const { location } = node.target
-      if (!location || !this.#seen.has(location)) {
-        const item = new QuerySelectorItem(node)
-        this.#response.push(item)
-        if (location) {
-          this.#seen.add(item.location)
+      if (!location) {
+        this.#response.push(new QuerySelectorItem(node))
+        continue
+      }
+      const seen = this.#seen.get(location)
+      if (seen === undefined) {
+        this.#seen.set(location, { index: this.#response.length, rank: locationRank(node) })
+        this.#response.push(new QuerySelectorItem(node))
+      } else {
+        // Replace the stored representation only with a more logical one for the same physical package.
+        const rank = locationRank(node)
+        if (rank > seen.rank) {
+          this.#response[seen.index] = new QuerySelectorItem(node)
+          seen.rank = rank
         }
       }
     }

test/lib/commands/query.js

@@ -160,6 +160,61 @@ t.test('linked node', async t => {
   t.matchSnapshot(joinedOutput(), 'should return linked node res')
 })
 
+t.test('linked strategy reports logical location, not store backing path', async t => {
+  /* linked layout: nopt (direct) symlinked to its store key, abbrev a transitive dep of nopt */
+  const linkedDir = {
+    prefixDir: {
+      node_modules: {
+        nopt: t.fixture('symlink', '.store/nopt-hash/node_modules/nopt'),
+        '.store': {
+          'nopt-hash': {
+            node_modules: {
+              nopt: {
+                'package.json': JSON.stringify({
+                  name: 'nopt',
+                  version: '7.2.1',
+                  dependencies: { abbrev: '^2.0.0' },
+                }),
+              },
+              abbrev: t.fixture('symlink', '../../abbrev-hash/node_modules/abbrev'),
+            },
+          },
+          'abbrev-hash': {
+            node_modules: {
+              abbrev: {
+                'package.json': JSON.stringify({ name: 'abbrev', version: '2.0.0' }),
+              },
+            },
+          },
+        },
+      },
+      'package.json': JSON.stringify({
+        name: 'project',
+        version: '1.0.0',
+        dependencies: { nopt: '^7.0.0' },
+      }),
+    },
+  }
+
+  await t.test(':root > * reports the logical link location', async t => {
+    const { npm, joinedOutput } = await loadMockNpm(t, linkedDir)
+    await npm.exec('query', [':root > *'])
+    const res = JSON.parse(joinedOutput())
+    t.equal(res.length, 1, 'returns a single result, not the store node duplicate')
+    t.equal(res[0].location, 'node_modules/nopt', 'reports the logical location')
+    t.match(res[0].realpath, /\.store/, 'realpath still resolves to the store')
+  })
+
+  await t.test(':root * keeps direct deps logical and transitive deps canonical', async t => {
+    const { npm, joinedOutput } = await loadMockNpm(t, linkedDir)
+    await npm.exec('query', [':root *'])
+    const byName = Object.fromEntries(JSON.parse(joinedOutput()).map(n => [n.name, n.location]))
+    t.equal(byName.nopt, 'node_modules/nopt', 'direct dep keeps its logical location')
+    t.equal(byName.abbrev, 'node_modules/.store/abbrev-hash/node_modules/abbrev',
+      'transitive dep reports its canonical store key, not a consumer symlink')
+  })
+})
+
 t.test('global', async t => {
   const { npm, joinedOutput } = await loadMockNpm(t, {
     config: {

workspaces/arborist/lib/query-selector-all.js

@@ -798,7 +798,8 @@ const hasParent = (node, compareNodes) => {
     // Only match if the node has a link whose parent is the compareNode. Without this check, nodes deep in the store (linked strategy) would incorrectly match as children of root via their fsParent chain.
     if (node.isTop && (node.resolveParent === compareNode)) {
       for (const link of node.linksIn) {
-        if (link.parent === compareNode) {
+        // A store-backing node (linked strategy) is reached through its logical Link, which matches via edgesIn below, so the store node itself is never a direct child.
+        if (link.parent === compareNode && !node.isInStore) {
           return true
         }
       }

workspaces/arborist/test/query-selector-all.js

@@ -1144,6 +1144,13 @@ t.test('linked strategy: :root > * excludes transitive deps and store nodes', as
     'nopt@7.2.1',
   ], ':root > * should only return direct dependencies, not transitive deps or store nodes')
 
+  // :root > * must return the logical Links, not the store backing nodes
+  const rawChildren = await q(tree, ':root > *')
+  t.same(rawChildren.map(n => n.location).sort(), [
+    'node_modules/ini',
+    'node_modules/nopt',
+  ], ':root > * returns the logical link locations, not .store backing paths')
+
   const rootDescendants = await querySelectorAll(tree, ':root *')
   t.same(rootDescendants, [
     'abbrev@2.0.0',