fs.glob: do not skip sibling entries after a seen child path

src/js/internal/fs/glob.ts

@@ -3,6 +3,8 @@
 //
 // Port of Node.js lib/internal/fs/glob.js (v26.3.0):
 //   https://github.com/nodejs/node/blob/50c35fea9e64d50ab3bb5f359e8523de89d6c798/lib/internal/fs/glob.js
+// plus "fs: fix glob early return skipping sibling entries" (v26.8.0):
+//   https://github.com/nodejs/node/commit/0ea2c86b5b70fdab268597e8c51040c703ee1328
 // backed by a vendored copy of minimatch (Node's deps/minimatch/index.js, ISC license):
 //   https://github.com/nodejs/node/blob/50c35fea9e64d50ab3bb5f359e8523de89d6c798/deps/minimatch/index.js
 // embedded verbatim below lazyMinimatch(); the vendored block is third-party
@@ -637,9 +639,6 @@ class Glob {
       const nSymlinks = new Set();
       for (const index of pattern.indexes) {
         // For each child, check potential patterns
-        if (this.#cache.seen(entryPath, pattern, index) || this.#cache.seen(entryPath, pattern, index + 1)) {
-          return;
-        }
         const current = pattern.at(index);
         const nextIndex = index + 1;
         const next = pattern.at(nextIndex);
@@ -856,9 +855,6 @@ class Glob {
       const nSymlinks = new Set();
       for (const index of pattern.indexes) {
         // For each child, check potential patterns
-        if (this.#cache.seen(entryPath, pattern, index) || this.#cache.seen(entryPath, pattern, index + 1)) {
-          return;
-        }
         const current = pattern.at(index);
         const nextIndex = index + 1;
         const next = pattern.at(nextIndex);

test/js/node/fs/glob.test.ts

@@ -5,6 +5,7 @@
 import { afterAll, beforeAll, describe, expect, it } from "bun:test";
 import { isWindows, tempDir, tempDirWithFiles } from "harness";
 import fs from "node:fs";
+import { sep } from "node:path";
 
 let tmp: string;
 beforeAll(() => {
@@ -295,3 +296,54 @@ describe("fs.globSync exclude with withFileTypes", () => {
     expect(String(inners[0].parentPath).replaceAll("\\", "/")).toEndWith("keep");
   });
 });
+
+// https://github.com/nodejs/node/issues/62897
+describe("a directory that is listed after one of its children was traversed", () => {
+  const src = { src: { helpers: { "a.ts": "", "b.ts": "" }, "index.ts": "" } };
+  const cases = [
+    {
+      // "a/**" matches "a" and every directory below it. "../*" lists the parent
+      // of each one: ".", "a", "a/b", "a/b/c", "a/c" and "a/c/d".
+      name: "'..' after '**'",
+      tree: { a: { b: { c: { d: {} } }, c: { d: { c: {} } }, x: "", z: "" } },
+      pattern: "a/**/../*",
+      expected: ["a", "a/b", "a/b/c", "a/b/c/d", "a/c", "a/c/d", "a/c/d/c", "a/x", "a/z"],
+    },
+    {
+      // The "./" pattern is traversed first, then "src/*.ts" lists "src".
+      // Entries are visited in sorted order, so "index.ts" must sort after "helpers".
+      name: "a pattern array with a './' pattern",
+      tree: src,
+      pattern: ["src/*.ts", "./src/helpers/*.ts"],
+      expected: ["src/helpers/a.ts", "src/helpers/b.ts", "src/index.ts"],
+    },
+    {
+      name: "a brace group with a './' alternative",
+      tree: src,
+      pattern: "{src/*.ts,./src/helpers/*.ts}",
+      expected: ["src/helpers/a.ts", "src/helpers/b.ts", "src/index.ts"],
+    },
+  ];
+
+  describe.each(cases)("$name", ({ tree, pattern, expected }) => {
+    const sorted = expected.map(path => path.replaceAll("/", sep)).sort();
+
+    it("fs.globSync", () => {
+      using dir = tempDir("fs-glob-seen", tree);
+      expect(fs.globSync(pattern, { cwd: String(dir) }).sort()).toEqual(sorted);
+    });
+
+    it("fs.promises.glob", async () => {
+      using dir = tempDir("fs-glob-seen", tree);
+      const paths = await Array.fromAsync(fs.promises.glob(pattern, { cwd: String(dir) }));
+      expect(paths.sort()).toEqual(sorted);
+    });
+
+    it("fs.glob", async () => {
+      using dir = tempDir("fs-glob-seen", tree);
+      const { promise, resolve, reject } = Promise.withResolvers<string[]>();
+      fs.glob(pattern, { cwd: String(dir) }, (err, paths) => (err ? reject(err) : resolve(paths)));
+      expect((await promise).sort()).toEqual(sorted);
+    });
+  });
+});

test/js/node/test/parallel/test-fs-glob.mjs

@@ -1,5 +1,6 @@
 import * as common from '../common/index.mjs';
 import tmpdir from '../common/tmpdir.js';
+import { spawnSync } from 'node:child_process';
 import { resolve, dirname, sep, relative, join, isAbsolute } from 'node:path';
 import { mkdir, writeFile, symlink, glob as asyncGlob } from 'node:fs/promises';
 import { glob, globSync, Dirent, chmodSync, writeFileSync, rmSync } from 'node:fs';
@@ -393,7 +394,7 @@ describe('fsPromises.glob - with file: URL as cwd', function() {
 
 const normalizeDirent = (dirent) => relative(fixtureDir, join(dirent.parentPath, dirent.name));
 // The call to `join()` with only one argument is important, as
-// it ensures that the proper path seperators are applied.
+// it ensures that the proper path separators are applied.
 const normalizePath = (path) => (isAbsolute(path) ? relative(fixtureDir, path) : join(path));
 
 describe('glob - withFileTypes', function() {
@@ -669,3 +670,78 @@ describe('globSync - ENOTDIR', function() {
     }
   });
 });
+
+describe('glob - seen cache', function() {
+  // Refs: https://github.com/nodejs/node/issues/62897
+  test('does not skip siblings after a seen child path', () => {
+    // The glob traversal used to return early from the children loop when a
+    // child path had already been seen through a different pattern context,
+    // silently dropping the remaining siblings. Whether the bug triggered
+    // depended on directory iteration order, so the child process pins the
+    // order by patching readdir before loading the glob implementation.
+    const script = `
+      const assert = require('node:assert');
+      const fs = require('node:fs');
+      const fsPromises = require('node:fs/promises');
+      const path = require('node:path');
+
+      const cwd = process.argv[1];
+      const a = path.join(cwd, 'a');
+      fs.mkdirSync(path.join(a, 'b', 'c', 'd'), { recursive: true });
+      fs.mkdirSync(path.join(a, 'c', 'd', 'c'), { recursive: true });
+      fs.writeFileSync(path.join(a, 'x'), '');
+      fs.writeFileSync(path.join(a, 'z'), '');
+
+      const originalReaddirSync = fs.readdirSync;
+      const originalReaddir = fsPromises.readdir;
+
+      const reorder = (target, entries) => {
+        if (!Array.isArray(entries) || target !== a) return entries;
+        const names = ['c', 'b', 'x', 'z'];
+        return names.map((name) => entries.find((entry) => entry.name === name))
+          .filter(Boolean);
+      };
+
+      fs.readdirSync = function(target, options) {
+        return reorder(target, originalReaddirSync.call(this, target, options));
+      };
+      fsPromises.readdir = async function(target, options) {
+        return reorder(target, await originalReaddir.call(this, target, options));
+      };
+
+      const { Glob } = require('internal/fs/glob');
+      const expected = ['a/b', 'a/c', 'a/x', 'a/z'];
+      const normalize = (results) =>
+        results.map((item) => item.replaceAll(path.sep, '/')).sort();
+
+      (async () => {
+        const syncResults = normalize(new Glob('a/**/../*', { cwd }).globSync());
+        for (const item of expected) {
+          assert.ok(syncResults.includes(item),
+                    \`missing \${item} from sync results: \${syncResults}\`);
+        }
+
+        const asyncResults = [];
+        for await (const item of new Glob('a/**/../*', { cwd }).glob()) {
+          asyncResults.push(item);
+        }
+        const normalized = normalize(asyncResults);
+        for (const item of expected) {
+          assert.ok(normalized.includes(item),
+                    \`missing \${item} from async results: \${normalized}\`);
+        }
+      })().catch((err) => {
+        console.error(err);
+        process.exitCode = 1;
+      });
+    `;
+
+    const seenDir = tmpdir.resolve('glob-seen');
+    const child = spawnSync(
+      process.execPath,
+      ['--expose-internals', '-e', script, seenDir],
+      { encoding: 'utf8' },
+    );
+    assert.strictEqual(child.status, 0, child.stderr || child.stdout);
+  });
+});