sotashimozono_obsidian-remo.../plugin/tests/pathUtils.test.ts
Souta b78471c616 fix(rpc): address PR #355 review — path-space mismatch + undefined guard
Multi-agent review caught a real regression in the daemon-reuse fix:

- C1 (CRITICAL): remotePath "~" → effectivePath "." →
  resolveRemotePath("." , home) = "/home/u/." but the daemon reports
  filepath.Abs(".") = "/home/u" via server.info → sameRemotePath
  (trailing-slash-only) = false → kill+redeploy on EVERY connect.
  Same for any "..", "//" in remotePath. The client (string-join) and
  daemon (filepath.Abs) lived in different path spaces.
- I1: reused.info.vaultRoot is typed string but an older/3rd-party
  daemon can omit it on the wire → undefined → sameRemotePath's
  .trim() threw TypeError → uncaught silent connect failure.
- I2: deploy() got the raw relative effectivePath as --vault-root →
  implicit daemon-cwd dependency.

Consolidated fix:
- ConnectionManager: compute the absolute vault root ONCE
  (resolveRemotePath(effectivePath, home)) and use it BOTH for the
  reuse compare AND as deploy()'s remoteVaultRoot — same path space
  on both sides, no cwd dependency (fixes C1 + I2 together).
- sameRemotePath: POSIX-normalise both sides (path.posix.normalize →
  collapses ./../ //, then strip trailing slash); null-safe
  (string|undefined); empty/blank never matches a real root, not
  even empty-vs-empty (always forces the safe redeploy). Fixes C1
  remainder + I1.
- haveRoot guarded `?? ''` at the call site; reused.close() catch
  now logger.warn (mirrors every other close in the file) instead of
  a silent swallow.
- JSDoc reworded: a spurious redeploy is SAFE but not cheap (full
  pkill + upload + restart) — normalisation exists to avoid firing
  it on legitimately-equal paths.

Tests: pathUtils.test.ts +6 (the "." / ~ loop case, ".."/"//"
collapse, empty-vs-empty=false, undefined-safe no-throw). The
startRpcSession reuse/mismatch glue is intentionally covered by the
now-exhaustive sameRemotePath unit suite + the connect e2e
(#351/#353) rather than a bespoke ssh/deployer mock harness —
disproportionate for thin control flow whose decision core is pure
and fully tested.

tsc clean, eslint clean, vitest 70 files / 1061 passed / 1 skipped.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
2026-05-18 00:28:26 +09:00

159 lines
6.2 KiB
TypeScript

import * as path from 'path';
import { describe, it, expect } from 'vitest';
import {
ensureTrailingSlash,
expandHome,
normalizeRemotePath,
posixJoin,
relativeTo,
sameRemotePath,
toLocalPath,
toRemotePath,
} from '../src/util/pathUtils';
describe('normalizeRemotePath', () => {
it('strips a leading "~/" so the path becomes home-relative for SFTP', () => {
expect(normalizeRemotePath('~/work/VaultDev/')).toBe('work/VaultDev');
expect(normalizeRemotePath('~/.config')).toBe('.config');
});
it('rewrites a bare "~" as "."', () => {
expect(normalizeRemotePath('~')).toBe('.');
});
it('leaves absolute paths untouched aside from trailing slashes', () => {
expect(normalizeRemotePath('/home/alice/vault/')).toBe('/home/alice/vault');
expect(normalizeRemotePath('/srv/vault')).toBe('/srv/vault');
});
it('trims trailing slashes but preserves the root "/"', () => {
expect(normalizeRemotePath('foo/bar///')).toBe('foo/bar');
expect(normalizeRemotePath('/')).toBe('/');
});
it('does not touch paths that contain "~" mid-string', () => {
expect(normalizeRemotePath('/home/~weird/stuff')).toBe('/home/~weird/stuff');
});
it('trims surrounding whitespace from user input', () => {
expect(normalizeRemotePath(' ~/work/VaultDev ')).toBe('work/VaultDev');
});
});
describe('path utility helpers', () => {
it('posixJoin joins with a single slash', () => {
expect(posixJoin('a', 'b', 'c')).toBe('a/b/c');
expect(posixJoin('/a/', '/b//', 'c')).toBe('/a/b/c');
});
it('relativeTo returns full path when base does not match', () => {
expect(relativeTo('/base', '/other/file.md')).toBe('/other/file.md');
});
it('relativeTo strips base with or without trailing slash', () => {
expect(relativeTo('/base', '/base/file.md')).toBe('file.md');
expect(relativeTo('/base/', '/base/file.md')).toBe('file.md');
});
it('ensureTrailingSlash appends only once', () => {
expect(ensureTrailingSlash('/tmp/work')).toBe('/tmp/work/');
expect(ensureTrailingSlash('/tmp/work/')).toBe('/tmp/work/');
});
it('toLocalPath uses platform-aware path join', () => {
const result = toLocalPath('/tmp/work', 'docs/a.md');
expect(result).toContain('docs');
expect(result.endsWith('docs/a.md') || result.endsWith('docs\\a.md')).toBe(true);
});
it('toRemotePath uses posix separator', () => {
expect(toRemotePath('/srv/vault', 'docs/a.md')).toBe('/srv/vault/docs/a.md');
});
it('expandHome expands "~/" using HOME', () => {
const originalHome = process.env.HOME;
const originalUserProfile = process.env.USERPROFILE;
try {
process.env.HOME = '/home/alice';
delete process.env.USERPROFILE;
// Use path.join for platform-agnostic comparison (Windows uses backslashes)
expect(expandHome('~/vault')).toBe(path.join('/home/alice', 'vault'));
} finally {
if (originalHome === undefined) delete process.env.HOME; else process.env.HOME = originalHome;
if (originalUserProfile === undefined) delete process.env.USERPROFILE; else process.env.USERPROFILE = originalUserProfile;
}
});
it('expandHome falls back to cwd-relative when neither HOME nor USERPROFILE is set', () => {
const originalHome = process.env.HOME;
const originalUserProfile = process.env.USERPROFILE;
try {
delete process.env.HOME;
delete process.env.USERPROFILE;
// home = '' → path.join('', 'vault') = 'vault'
expect(expandHome('~/vault')).toBe(path.join('', 'vault'));
} finally {
if (originalHome === undefined) delete process.env.HOME; else process.env.HOME = originalHome;
if (originalUserProfile === undefined) delete process.env.USERPROFILE; else process.env.USERPROFILE = originalUserProfile;
}
});
it('expandHome leaves non-tilde paths unchanged', () => {
expect(expandHome('/srv/vault')).toBe('/srv/vault');
});
});
describe('sameRemotePath (RPC daemon vault-root validation)', () => {
it('equal paths match', () => {
expect(sameRemotePath('/home/souta/work', '/home/souta/work')).toBe(true);
});
it('trailing slash is ignored on either side', () => {
expect(sameRemotePath('/home/souta/work/', '/home/souta/work')).toBe(true);
expect(sameRemotePath('/home/souta/work', '/home/souta/work/')).toBe(true);
expect(sameRemotePath('/home/souta/work//', '/home/souta/work')).toBe(true);
});
it('surrounding whitespace is ignored', () => {
expect(sameRemotePath(' /home/souta/work ', '/home/souta/work')).toBe(true);
});
it('a deeper/old root does NOT match the wanted root (the field bug)', () => {
// Stale daemon rooted at the deleted VaultDev vs profile now
// pointing at ~/work → must NOT reuse, must redeploy.
expect(sameRemotePath('/home/souta/work/VaultDev', '/home/souta/work')).toBe(false);
});
it('different paths do not match', () => {
expect(sameRemotePath('/home/souta/a', '/home/souta/b')).toBe(false);
expect(sameRemotePath('', '/home/souta/work')).toBe(false);
});
it('root path "/" is preserved (not stripped to empty)', () => {
expect(sameRemotePath('/', '/')).toBe(true);
expect(sameRemotePath('/', '')).toBe(false);
});
it('normalises "." — remotePath "~" case that caused the redeploy loop', () => {
// resolveRemotePath('.', '/home/souta') = '/home/souta/.'
// daemon filepath.Abs('.') reports '/home/souta' → must MATCH now.
expect(sameRemotePath('/home/souta/.', '/home/souta')).toBe(true);
});
it('normalises ".." and collapses "//"', () => {
expect(sameRemotePath('/home/souta/work/../work', '/home/souta/work')).toBe(true);
expect(sameRemotePath('/home//souta/work', '/home/souta/work')).toBe(true);
});
it('empty-vs-empty is NOT a match (missing root always redeploys)', () => {
expect(sameRemotePath('', '')).toBe(false);
expect(sameRemotePath(' ', '/x')).toBe(false);
});
it('undefined inputs are safe (no throw) and never match', () => {
expect(() => sameRemotePath(undefined, '/home/souta/work')).not.toThrow();
expect(sameRemotePath(undefined, '/home/souta/work')).toBe(false);
expect(sameRemotePath('/home/souta/work', undefined)).toBe(false);
expect(sameRemotePath(undefined, undefined)).toBe(false);
});
});