fix(cli): store resolved (non-canonical) path, compare via canonicalizePath (#1003 CI)

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`.
This commit is contained in:
azizur1992 2026-04-21 09:26:28 +01:00
parent c5eceba0cb
commit 608299c22f

View file

@ -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);