From 608299c22fc0676cef7b766e9592383948eb93e5 Mon Sep 17 00:00:00 2001
From: azizur1992 <azizur100389@gmail.com>
Date: Tue, 21 Apr 2026 09:26:28 +0100
Subject: [PATCH] fix(cli): store resolved (non-canonical) path, compare via
 canonicalizePath (#1003 CI)
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

Follow-up to c5eceba0. The previous commit canonicalised the repo path
at BOTH write-time AND compare-time in registerRepo — that expanded
Windows 8.3 short names (RUNNER~1) to long names (runneradmin) when
storing `entry.path`. Pre-existing #829 unit tests that assert
`path.resolve(err.existingPath) === path.resolve(tmpPath)` then broke
because `tmpPath` is still short-form (path.resolve doesn't expand
8.3) while `entry.path` was long-form (canonicalizePath does).

Fix: split storage from comparison.
- entry.path stores `path.resolve(repoPath)` — whatever form the
  caller passed. `list` output and error messages show the path the
  user typed.
- All compare points (existing-entry lookup in registerRepo, the
  collision guard, unregisterRepo, resolveRegistryEntry path tier)
  canonicalise BOTH sides via `canonicalizePath`. That is where the
  /var ↔ /private/var and RUNNER~1 ↔ runneradmin divergence actually
  matters.

Net effect: storage is tolerant (preserves user input), matching is
strict (canonical-vs-canonical). Pre-existing #829 tests stay green
because `err.existingPath` is unchanged from what `path.resolve` gives
back; the cross-platform CI failure from #1003 stays fixed because
every comparison path goes through `canonicalizePath`.
---
 gitnexus/src/storage/repo-manager.ts | 37 +++++++++++++++++++---------
 1 file changed, 25 insertions(+), 12 deletions(-)

diff --git a/gitnexus/src/storage/repo-manager.ts b/gitnexus/src/storage/repo-manager.ts
index 572e8c783..d7bf47228 100644
--- a/gitnexus/src/storage/repo-manager.ts
+++ b/gitnexus/src/storage/repo-manager.ts
@@ -389,23 +389,33 @@ export const registerRepo = async (
   meta: RepoMeta,
   opts?: RegisterRepoOptions,
 ): Promise<string> => {
-  // Canonicalise the caller's path up-front (#1003 review) — expands
-  // macOS /var → /private/var and Windows 8.3 → long-name so the
-  // registry entry stays matchable by `resolveRegistryEntry` regardless
-  // of which form the caller hands us.
-  const resolved = canonicalizePath(repoPath);
+  // Preserve the caller's chosen path form in the registry — don't
+  // canonicalise at write time. This matters for two reasons:
+  //   1. `list` and error messages show the path the user actually
+  //      knows (e.g. the 8.3 short form they typed), not a runtime-
+  //      resolved long form they've never seen.
+  //   2. Keeps pre-existing #829 test assertions that compare
+  //      `err.existingPath` against `path.resolve(tmpPath)` stable.
+  // Canonicalisation is applied at COMPARE points only (see below),
+  // which is where the cross-platform divergence actually matters.
+  const resolved = path.resolve(repoPath);
   const { storagePath } = getStoragePaths(resolved);
 
+  // Canonical form used strictly for comparison — `realpathSync.native`
+  // expands macOS /var → /private/var and Windows 8.3 → long-name,
+  // falling back to `path.resolve` when the path doesn't exist.
+  const canonicalInput = canonicalizePath(repoPath);
+
   const entries = await readRegistry();
   const existingIdx = entries.findIndex((e) => {
     // Canonicalise the STORED entry too so pre-canonicalisation
-    // registries (written by older versions, or the same version before
-    // this review fix) still match correctly. `canonicalizePath` falls
-    // back to `path.resolve` when the path no longer exists on disk, so
-    // stale entries that have been rm'd externally still resolve to a
-    // stable key instead of throwing.
+    // registries (written by older versions, or paths passed in a
+    // different form) still match correctly. `canonicalizePath` falls
+    // back to `path.resolve` when the path no longer exists on disk,
+    // so stale entries that have been rm'd externally still resolve
+    // to a stable key instead of throwing.
     const a = canonicalizePath(e.path);
-    const b = resolved;
+    const b = canonicalInput;
     return process.platform === 'win32' ? a.toLowerCase() === b.toLowerCase() : a === b;
   });
   const existing = existingIdx >= 0 ? entries[existingIdx] : null;
@@ -439,11 +449,14 @@ export const registerRepo = async (
   // messages and list output #829 ships).
   const explicitName = opts?.name !== undefined || isPreservedAlias;
   if (explicitName && !opts?.allowDuplicateName) {
+    // Compare canonical-vs-canonical here too so `/var/foo` and
+    // `/private/var/foo` (same repo, different form) aren't treated as
+    // two colliding paths.
     const collidingEntry = entries.find(
       (e, i) =>
         i !== existingIdx &&
         e.name.toLowerCase() === name.toLowerCase() &&
-        canonicalizePath(e.path) !== resolved,
+        canonicalizePath(e.path) !== canonicalInput,
     );
     if (collidingEntry) {
       throw new RegistryNameCollisionError(name, collidingEntry.path, resolved);