fix(#4341): reference-count the git-config sandbox the three guard suites share (#4389)

* fix(#4341): reference-count the git-config sandbox the three guard suites share

node:test evaluates every describe body during collection, before any test
runs, so the three isolateGlobalGitConfig() calls happened back to back and
each captured the PREVIOUS call's temp path as its "original": A captured
undefined, B captured A's path, D captured B's. Suite A's after() fires first
and restored undefined — deleting GIT_CONFIG_GLOBAL outright — so suites B and
D ran against the developer's real ~/.gitconfig for the rest of the file. A
also cleanup()'d a directory B still pointed at.

One sandbox now, with the true original captured once and released when the
last holder lets go; each returned restorer is idempotent, so an extra call
cannot release someone else's hold. The three call sites are unchanged.

Reproduced with a global core.hooksPath (via a fixture HOME carrying a
.gitconfig, so the developer's real one is never touched):

  next:   ℹ pass 291  ℹ fail 7   — "core.hooksPath is set to ...; a hook
                                    written to .../pre-commit would never run"
  branch: ℹ pass 296  ℹ fail 3

The 3 that remain are two suites (pr-subrepo, #3776 query commit --files) that
never called isolateGlobalGitConfig at all — the same class, a different gap,
and outside this issue's scope. Noted on the PR.

D0 is the regression guard and is deterministic on every lane: suite A's
after() runs before this suite's tests, so on the old helper GIT_CONFIG_GLOBAL
is already gone by then regardless of what the host's git config contains —
which is what makes it fail on CI, where the core.hooksPath that exposed the
defect is absent. Verified: full-file run reds on the old helper, greens on the
new one.

* test(#4341): clean filtered gitconfig sandboxes on exit

* test(#4341): retain exit cleanup until release succeeds

tests/commands.test.cjs

@@ -3069,15 +3069,57 @@ describe('check-commit command', () => {
 // `git config --get` returns any non-empty local value (same refusal), and an
 // empty local value makes rev-parse --git-path hooks resolve to `./` — not
 // `.git/hooks`. Returns a restore function for the suite's after().
+//
+// #4341: reference-counted, because three suites call this from their DESCRIBE
+// bodies and node:test evaluates every describe body during collection, before
+// any test runs. The previous version captured `prev` per call, so the three
+// calls chained (A captured undefined, B captured A's temp path, D captured
+// B's) and the FIRST after() to fire — A's — restored `undefined`, deleting
+// GIT_CONFIG_GLOBAL outright and dropping suites B and D onto the developer's
+// real ~/.gitconfig for the rest of the run. It also cleanup()'d A's dir while
+// B still pointed at it. That is the "passes alone, fails in the full run"
+// signature: 15/15 with --test-name-pattern, 8/15 in the whole file, on any
+// machine with a global core.hooksPath.
+//
+// One sandbox, the TRUE original captured once, released when the last holder
+// lets go. Each returned restorer is idempotent, so an extra call cannot
+// release someone else's hold.
+let _gitConfigIsolation = null;
+
 function isolateGlobalGitConfig() {
-  const dir = createTempDir('gsd-3901-gitconfig-');
-  const prev = process.env.GIT_CONFIG_GLOBAL;
-  process.env.GIT_CONFIG_GLOBAL = path.join(dir, 'global.gitconfig');
-  fs.writeFileSync(process.env.GIT_CONFIG_GLOBAL, '');
+  if (_gitConfigIsolation) {
+    _gitConfigIsolation.refs += 1;
+  } else {
+    const dir = createTempDir('gsd-3901-gitconfig-');
+    const file = path.join(dir, 'global.gitconfig');
+    fs.writeFileSync(file, '');
+    // A --test-name-pattern filter can collect a suite without running its
+    // tests/after hook, leaving one reference unreleased. Keep normal cleanup
+    // reference-counted, with a process-exit fallback for that filtered run.
+    const cleanupOnExit = () => cleanup(dir);
+    _gitConfigIsolation = {
+      dir,
+      file,
+      prev: process.env.GIT_CONFIG_GLOBAL,
+      refs: 1,
+      cleanupOnExit,
+    };
+    process.once('exit', cleanupOnExit);
+    process.env.GIT_CONFIG_GLOBAL = file;
+  }
+  let released = false;
   return () => {
+    if (released) return;
+    released = true;
+    if (!_gitConfigIsolation) return;
+    _gitConfigIsolation.refs -= 1;
+    if (_gitConfigIsolation.refs > 0) return;
+    const { prev, dir, cleanupOnExit } = _gitConfigIsolation;
+    _gitConfigIsolation = null;
     if (prev === undefined) delete process.env.GIT_CONFIG_GLOBAL;
     else process.env.GIT_CONFIG_GLOBAL = prev;
     cleanup(dir);
+    process.removeListener('exit', cleanupOnExit);
   };
 }
 
@@ -3383,6 +3425,27 @@ describe('commit-docs-guard real git commit wiring (#3588 D1-D3)', () => {
   // #3901: the D suite's premise is the hook firing from .git/hooks/pre-commit
   // — a hostile global core.hooksPath broke its beforeEach identically.
   const restoreGitConfigD = isolateGlobalGitConfig();
+
+  // #4341 regression guard, and deterministic on every lane: suite A's after()
+  // fires before this suite's tests, so on the pre-#4341 helper
+  // GIT_CONFIG_GLOBAL is already GONE by the time this runs — regardless of
+  // what the host's real git config happens to contain. That is what makes
+  // this catch the release-ordering defect on CI, where the core.hooksPath
+  // that exposed it is absent.
+  test('D0: the git-config sandbox is still in effect after the earlier suites released theirs (#4341)', () => {
+    assert.ok(
+      process.env.GIT_CONFIG_GLOBAL,
+      'GIT_CONFIG_GLOBAL must still be set — an earlier suite released the shared isolation',
+    );
+    assert.ok(
+      process.env.GIT_CONFIG_GLOBAL.includes('gsd-3901-gitconfig-'),
+      `GIT_CONFIG_GLOBAL must point into the sandbox, got: ${process.env.GIT_CONFIG_GLOBAL}`,
+    );
+    assert.ok(
+      fs.existsSync(process.env.GIT_CONFIG_GLOBAL),
+      'the sandbox file must still exist — an earlier suite cleaned up a directory it did not own',
+    );
+  });
   after(restoreGitConfigD);
 
   beforeEach(() => {