fix(terminal): stop the shim PATH scrub deleting a legitimate directory

The previous round collapsed '..' lexically before classifying a PATH entry.
That is not the same as resolving it: when <shim>/posix is a symlink,
<shim>/posix/../posix lands elsewhere, so a legitimate directory was
classified as the shim and removed, leaving git unresolvable. Reproduced with
a symlinked shim/posix and a real git behind it.

Resolving for real is not available here either -- this env is also built for
remote and WSL panes whose paths name nothing on the local filesystem -- so the
classifier is lexical again, deliberately. A '..' spelling that slips through
costs nothing at runtime: that directory holds the pass-through tombstone, and
the tombstone excludes its own directory by -ef, so the lookup still reaches
the real git.

Separately, pathEntrySpellings can only enumerate one added separator, so a
captured directory spelled with two or more survived the literal removal. The
split filter now also compares separator-stripped forms, which covers any
number. Both changes are mutation-tested.

Co-authored-by: Orca <help@stably.ai>

src/main/pty/legacy-terminal-shim-dir.test.ts

@@ -332,21 +332,38 @@ describe('legacy terminal shim neutralization', () => {
     }
   })
 
-  it('treats a shim directory reached through .. as a shim PATH entry', () => {
-    // Why: a plain suffix test missed `<root>/orca-terminal-attribution/posix/../posix`, so the
-    // entry survived the scrub and the shim stayed reachable.
+  it('does not collapse .. when classifying a shim PATH entry', () => {
+    // Why deliberately not collapsed: collapsing `..` textually is not resolving it. If
+    // `<shim>/posix` is a symlink, `<shim>/posix/../posix` lands somewhere else, and classifying
+    // it as the shim directory deletes a legitimate PATH entry and leaves git unresolvable.
+    // Resolving for real is not available here either: this env is also built for remote and WSL
+    // panes whose paths do not exist on the local filesystem.
     expect(isLegacyTerminalShimPathEntry('/tmp/old/orca-terminal-attribution/posix/../posix')).toBe(
-      true
-    )
-    expect(isLegacyTerminalShimPathEntry('/tmp/old/orca-terminal-attribution/win32/../win32')).toBe(
-      true
-    )
-    expect(isLegacyTerminalShimPathEntry('/tmp/old/orca-terminal-attribution/posix/../other')).toBe(
       false
     )
+    expect(isLegacyTerminalShimPathEntry('/tmp/old/orca-terminal-attribution/posix')).toBe(true)
+    expect(isLegacyTerminalShimPathEntry('/tmp/old/orca-terminal-attribution/win32//')).toBe(true)
     expect(isLegacyTerminalShimPathEntry('/usr/local/bin')).toBe(false)
   })
 
+  it('strips a captured shim directory spelled with repeated separators', () => {
+    // Why: pathEntrySpellings can only enumerate one added separator, so an entry repeating it
+    // survived the literal removal and kept the captured directory on PATH.
+    const posixEnv = {
+      PATH: '/custom/elsewhere///:/usr/bin',
+      ORCA_ATTRIBUTION_SHIM_DIR: '/custom/elsewhere'
+    }
+    stripLegacyTerminalShimEnv(posixEnv, 'linux')
+    expect(posixEnv.PATH).toBe('/usr/bin')
+
+    const windowsEnv = {
+      Path: 'C:\\Custom\\Else\\\\;C:\\Windows',
+      ORCA_ATTRIBUTION_SHIM_DIR: 'C:\\Custom\\Else'
+    }
+    stripLegacyTerminalShimEnv(windowsEnv, 'win32')
+    expect(windowsEnv.Path).toBe('C:\\Windows')
+  })
+
   itOnPosix('does not fall back to the cwd when every PATH entry is filtered out', () => {
     // Why: with the shim dir as the only entry the cleaned PATH is empty; without the
     // path_entry_kept guard the lookup runs against that empty PATH and finds a cwd-local git.

src/main/pty/legacy-terminal-shim-dir.ts

@@ -1,5 +1,5 @@
 import { chmodSync, existsSync, mkdirSync, renameSync, rmSync, writeFileSync } from 'node:fs'
-import { join, normalize } from 'node:path'
+import { join } from 'node:path'
 import {
   readVerifiedShebangInterpreter,
   renderLegacyTerminalPosixTombstone,
@@ -146,7 +146,7 @@ function writeFileAtomically(filePath: string, contents: string, mode: number):
 // literal compare itself has to stay: a shim path may contain the PATH delimiter, which splitting
 // would fragment.
 function pathEntrySpellings(dir: string, windows: boolean): string[] {
-  const base = dir.replace(/[\\/]+$/, '')
+  const base = stripTrailingSeparators(dir)
   if (!base) {
     return [dir]
   }
@@ -157,20 +157,40 @@ function pathEntrySpellings(dir: string, windows: boolean): string[] {
   return [...spellings]
 }
 
+// Why purely lexical, and why `..` is deliberately not collapsed: collapsing it textually is not
+// the same as resolving it. If `<shim>/posix` is a symlink, `<shim>/posix/../posix` resolves
+// somewhere else entirely, and treating it as the shim directory deletes a legitimate PATH entry
+// and leaves git unresolvable. Resolving for real is not an option either: this env is also built
+// for remote and WSL panes, where these paths do not name anything on the local filesystem.
+// A `..` spelling that does slip through costs nothing at runtime -- the directory now holds the
+// pass-through tombstone, and the tombstone excludes its own directory by -ef, so the lookup still
+// reaches the real git.
 export function isLegacyTerminalShimPathEntry(entry: string): boolean {
-  // Why normalize first: a `.../orca-terminal-attribution/posix/../posix` spelling names the shim
-  // directory but fails a plain suffix test, so the entry survived the scrub and the shim stayed
-  // reachable. normalize collapses `..` textually, which is what a suffix test needs.
-  const normalized = normalize(entry.replaceAll('\\', '/'))
-    .replaceAll('\\', '/')
-    .replace(/\/+$/, '')
-    .toLowerCase()
+  const normalized = stripTrailingSeparators(entry.replaceAll('\\', '/')).toLowerCase()
   return (
     normalized.endsWith(`/${LEGACY_SHIM_ROOT_DIR}/posix`) ||
     normalized.endsWith(`/${LEGACY_SHIM_ROOT_DIR}/win32`)
   )
 }
 
+// Why: `pathEntrySpellings` can only enumerate one added separator, so an entry repeating it
+// survived the literal removal. Comparing separator-stripped forms covers any number of them.
+function stripTrailingSeparators(value: string): string {
+  return value.replace(/[\\/]+$/, '')
+}
+
+function namesCapturedShimDir(entry: string, shimDirs: string[], windows: boolean): boolean {
+  const candidate = stripTrailingSeparators(entry)
+  return shimDirs.some((shimDir) => {
+    const target = stripTrailingSeparators(shimDir)
+    return target
+      ? windows
+        ? candidate.toLowerCase() === target.toLowerCase()
+        : candidate === target
+      : false
+  })
+}
+
 export function stripLegacyTerminalShimEnv(
   env: Record<string, string>,
   platform: NodeJS.Platform = process.platform
@@ -210,7 +230,12 @@ export function stripLegacyTerminalShimEnv(
     )
     const cleaned = withoutExplicitDirs
       .split(delimiter)
-      .filter((entry) => entry && !isLegacyTerminalShimPathEntry(entry))
+      .filter(
+        (entry) =>
+          entry &&
+          !isLegacyTerminalShimPathEntry(entry) &&
+          !namesCapturedShimDir(entry, explicitShimDirs, windows)
+      )
       .join(delimiter)
     if (cleaned) {
       env[pathKey] = cleaned