diff --git a/src/logic/gitignore-manager.ts b/src/logic/gitignore-manager.ts index 9552057..e51bb81 100644 --- a/src/logic/gitignore-manager.ts +++ b/src/logic/gitignore-manager.ts @@ -2,6 +2,7 @@ import ignore, { Ignore } from 'ignore'; import { App } from 'obsidian'; import { GitServiceInterface } from '../services/git-service-interface'; import { logger } from '../utils/logger'; +import { readLocalSymlinkTarget } from '../utils/symlink'; export class GitignoreManager { private readonly app: App; @@ -95,6 +96,11 @@ export class GitignoreManager { } } for (const subFolder of listing.folders) { + // A folder that's actually a symlink (e.g. a shared folder linked in from + // elsewhere) is a single git blob on the remote, not a real tree — walking + // into it would scan an unrelated directory structure and produce bogus + // .gitignore lookups against paths that don't exist in this repo. + if (readLocalSymlinkTarget(this.app, subFolder) !== null) continue; await this.scanDir(subFolder, out); } } catch { /* adapter.list may be unavailable in some environments */ } diff --git a/src/ui/SyncStatusView.ts b/src/ui/SyncStatusView.ts index b47ca9c..232847d 100644 --- a/src/ui/SyncStatusView.ts +++ b/src/ui/SyncStatusView.ts @@ -8,6 +8,7 @@ import { renderActionBar } from './components/ActionBar'; import { renderFileItem, statusMeta, type FileItemCallbacks } from './components/FileListItem'; import { ICONS } from './components/icons'; import { isBinaryPath, contentsEqual } from '../utils/path'; +import { readLocalSymlinkTarget } from '../utils/symlink'; export const SYNC_STATUS_VIEW_TYPE = 'sync-status-view'; @@ -370,6 +371,14 @@ export class SyncStatusView extends ItemView { } for (const folder of listing.folders) { if (folder === '.git' || folder.endsWith('/.git')) continue; + // A symlinked folder is a single blob on the remote, not a real tree — + // walking into it would scan whatever unrelated directory it points at. + // Track the link itself (same as a hidden file) so push/pull can still + // handle it via the existing symlink machinery, without recursing. + if (readLocalSymlinkTarget(this.app, folder) !== null) { + if (this.isHidden(folder)) result.push(folder); + continue; + } await this.recursiveScan(folder, result); } } catch { /* adapter may not support listing */ } diff --git a/src/utils/symlink.ts b/src/utils/symlink.ts index 5b4de51..3779c6c 100644 --- a/src/utils/symlink.ts +++ b/src/utils/symlink.ts @@ -14,16 +14,18 @@ import { logger } from './logger'; // builtins (they don't exist in the mobile bundle). interface NodeFs { lstatSync(p: string): { isSymbolicLink(): boolean }; + statSync(p: string): { isDirectory(): boolean }; readlinkSync(p: string): string; existsSync(p: string): boolean; mkdirSync(p: string, opts: { recursive: boolean }): void; rmSync(p: string, opts: { force: boolean }): void; - symlinkSync(target: string, p: string): void; + symlinkSync(target: string, p: string, type?: string): void; } interface NodePath { join(...parts: string[]): string; dirname(p: string): string; + isAbsolute(p: string): boolean; } type NodeRequire = (id: string) => unknown; @@ -88,7 +90,7 @@ export function createLocalSymlink(app: App, vaultPath: string, target: string): if (mods.fs.existsSync(abs) || isSymlink(mods.fs, abs)) { mods.fs.rmSync(abs, { force: true }); } - mods.fs.symlinkSync(target, abs); + mods.fs.symlinkSync(target, abs, symlinkType(mods, abs, target)); return true; } catch (e) { logger.warn(`Failed to create local symlink ${vaultPath} -> ${target}`, e); @@ -103,3 +105,17 @@ function isSymlink(fs: NodeFs, abs: string): boolean { return false; } } + +// Node's `type` hint to symlinkSync is a no-op on POSIX but required on +// Windows: omitting it defaults to 'file', which produces a broken link +// whenever the target is actually a directory (as with a symlinked folder). +function symlinkType(mods: { fs: NodeFs; path: NodePath }, abs: string, target: string): 'file' | 'dir' { + try { + const resolvedTarget = mods.path.isAbsolute(target) + ? target + : mods.path.join(mods.path.dirname(abs), target); + return mods.fs.statSync(resolvedTarget).isDirectory() ? 'dir' : 'file'; + } catch { + return 'file'; + } +} diff --git a/tests/logic/gitignore-manager.test.ts b/tests/logic/gitignore-manager.test.ts index af9b393..3dd982b 100644 --- a/tests/logic/gitignore-manager.test.ts +++ b/tests/logic/gitignore-manager.test.ts @@ -3,8 +3,10 @@ import { describe, it, expect, vi, beforeEach, Mocked } from 'vitest'; import { GitignoreManager } from '../../src/logic/gitignore-manager'; import { App, DataAdapter } from 'obsidian'; import { GitServiceInterface } from '../../src/services/git-service-interface'; +import { readLocalSymlinkTarget } from '../../src/utils/symlink'; vi.mock('obsidian'); +vi.mock('../../src/utils/symlink', () => ({ readLocalSymlinkTarget: vi.fn() })); describe('GitignoreManager', () => { let manager: GitignoreManager; @@ -14,6 +16,7 @@ describe('GitignoreManager', () => { beforeEach(() => { vi.clearAllMocks(); + vi.mocked(readLocalSymlinkTarget).mockReturnValue(null); const mockAdapter = { exists: vi.fn(), @@ -272,5 +275,24 @@ describe('GitignoreManager', () => { expect(adapter.list).toHaveBeenCalledWith(''); expect(adapter.list).toHaveBeenCalledWith('.claude'); }); + + it('should not recurse into a folder that is actually a symlink', async () => { + // "linked" is a directory symlink (e.g. a shared skills folder) pointing + // outside the repo; walking into it would scan an unrelated tree and + // produce bogus .gitignore lookups for paths that don't exist remotely. + vi.mocked(adapter.list).mockImplementation((dir: string) => { + if (dir === '' || dir === undefined) { + return Promise.resolve({ files: ['.gitignore'], folders: ['linked'] }); + } + return Promise.resolve({ files: ['linked/nested/.gitignore'], folders: ['linked/nested'] }); + }); + vi.mocked(readLocalSymlinkTarget).mockImplementation((_app, path) => path === 'linked' ? '/some/other/dir' : null); + vi.mocked(adapter.read).mockResolvedValue('*.secret'); + + await manager.loadGitignores(); + + expect(adapter.list).toHaveBeenCalledWith(''); + expect(adapter.list).not.toHaveBeenCalledWith('linked'); + }); }); }); diff --git a/tests/setup.ts b/tests/setup.ts index d7f73dc..fb009ad 100644 --- a/tests/setup.ts +++ b/tests/setup.ts @@ -57,6 +57,10 @@ export const TFile = class {}; export const requestUrl = vi.fn(); export const setTooltip = vi.fn(); export const setIcon = vi.fn(); +export const Platform = { isDesktopApp: true }; +export const FileSystemAdapter = class { + getBasePath() { return '/mock/path'; } +}; vi.mock('obsidian', () => ({ Plugin, @@ -71,4 +75,6 @@ vi.mock('obsidian', () => ({ requestUrl, setTooltip, setIcon, + Platform, + FileSystemAdapter, })); diff --git a/tests/utils/symlink.test.ts b/tests/utils/symlink.test.ts new file mode 100644 index 0000000..d52f8bb --- /dev/null +++ b/tests/utils/symlink.test.ts @@ -0,0 +1,126 @@ +import { describe, it, expect, vi, beforeEach, afterEach } from 'vitest'; +import { App, FileSystemAdapter, Platform } from 'obsidian'; +import { canUseRealSymlinks, readLocalSymlinkTarget, createLocalSymlink } from '../../src/utils/symlink'; + +vi.mock('obsidian'); + +interface FakeFs { + lstatSync: ReturnType; + statSync: ReturnType; + readlinkSync: ReturnType; + existsSync: ReturnType; + mkdirSync: ReturnType; + rmSync: ReturnType; + symlinkSync: ReturnType; +} + +describe('symlink utils', () => { + let app: App; + let fakeFs: FakeFs; + const fakePath = { + join: (...parts: string[]) => parts.join('/'), + dirname: (p: string) => p.split('/').slice(0, -1).join('/'), + isAbsolute: (p: string) => p.startsWith('/'), + }; + + beforeEach(() => { + Platform.isDesktopApp = true; + + app = new App(); + app.vault.adapter = new FileSystemAdapter(); + + fakeFs = { + lstatSync: vi.fn().mockReturnValue({ isSymbolicLink: () => false }), + statSync: vi.fn().mockReturnValue({ isDirectory: () => false }), + readlinkSync: vi.fn().mockReturnValue(''), + existsSync: vi.fn().mockReturnValue(false), + mkdirSync: vi.fn(), + rmSync: vi.fn(), + symlinkSync: vi.fn(), + }; + + (window as unknown as { require: (id: string) => unknown }).require = (id: string) => + id === 'fs' ? fakeFs : fakePath; + }); + + afterEach(() => { + delete (window as unknown as { require?: unknown }).require; + }); + + describe('canUseRealSymlinks', () => { + it('is true on desktop with a FileSystemAdapter', () => { + expect(canUseRealSymlinks(app)).toBe(true); + }); + + it('is false when not a desktop app', () => { + Platform.isDesktopApp = false; + expect(canUseRealSymlinks(app)).toBe(false); + }); + + it('is false when the adapter is not a FileSystemAdapter', () => { + app.vault.adapter = {} as FileSystemAdapter; + expect(canUseRealSymlinks(app)).toBe(false); + }); + }); + + describe('readLocalSymlinkTarget', () => { + it('returns the raw target for a symlink', () => { + fakeFs.lstatSync.mockReturnValue({ isSymbolicLink: () => true }); + fakeFs.readlinkSync.mockReturnValue('../target'); + + expect(readLocalSymlinkTarget(app, 'skills/blog-master')).toBe('../target'); + }); + + it('returns null for a non-symlink path', () => { + fakeFs.lstatSync.mockReturnValue({ isSymbolicLink: () => false }); + expect(readLocalSymlinkTarget(app, 'notes/plain.md')).toBeNull(); + }); + + it('returns null when real symlinks are unavailable', () => { + Platform.isDesktopApp = false; + expect(readLocalSymlinkTarget(app, 'skills/blog-master')).toBeNull(); + }); + }); + + describe('createLocalSymlink', () => { + it('creates a "dir" symlink when the target resolves to a directory', () => { + fakeFs.statSync.mockReturnValue({ isDirectory: () => true }); + + const result = createLocalSymlink(app, 'skills/blog-master', '../blog/src'); + + expect(result).toBe(true); + expect(fakeFs.symlinkSync).toHaveBeenCalledWith('../blog/src', expect.any(String), 'dir'); + }); + + it('creates a "file" symlink when the target resolves to a file', () => { + fakeFs.statSync.mockReturnValue({ isDirectory: () => false }); + + createLocalSymlink(app, 'notes/link.md', '../shared/note.md'); + + expect(fakeFs.symlinkSync).toHaveBeenCalledWith('../shared/note.md', expect.any(String), 'file'); + }); + + it('falls back to "file" when the target cannot be stat-ed (dangling link)', () => { + fakeFs.statSync.mockImplementation(() => { throw new Error('ENOENT'); }); + + createLocalSymlink(app, 'notes/link.md', '../missing.md'); + + expect(fakeFs.symlinkSync).toHaveBeenCalledWith('../missing.md', expect.any(String), 'file'); + }); + + it('removes an existing file or stale link before creating the new one', () => { + fakeFs.existsSync.mockReturnValue(true); + + createLocalSymlink(app, 'notes/link.md', '../shared/note.md'); + + expect(fakeFs.rmSync).toHaveBeenCalledWith(expect.any(String), { force: true }); + expect(fakeFs.symlinkSync).toHaveBeenCalled(); + }); + + it('returns false when real symlinks are unavailable', () => { + Platform.isDesktopApp = false; + expect(createLocalSymlink(app, 'notes/link.md', '../shared/note.md')).toBe(false); + expect(fakeFs.symlinkSync).not.toHaveBeenCalled(); + }); + }); +});