From 4c8896b6fa2bc5eae40d67168e171955a9234ed9 Mon Sep 17 00:00:00 2001 From: ClaudiaFang Date: Mon, 13 Jul 2026 12:52:09 +0000 Subject: [PATCH] fix: symlinked directories no longer break pull discovery MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit A folder that's actually an OS symlink (e.g. a shared skills folder linked in from elsewhere) is a single git blob on the remote, not a real tree. Local scanning code that walked every subfolder returned by adapter.list() didn't know this, so it recursed straight into whatever unrelated directory the link pointed at: - GitignoreManager.scanDir() picked up bogus nested .gitignore paths under the linked folder and tried to fetch them from the remote repo, spamming 404s on every refresh (this matches the console errors in the issue's screenshot). - SyncStatusView's hidden-file scan recursed the same way instead of registering the symlink itself as a trackable path, so the folder never resolved to anything pullable — it just showed up as permanently "remote only". Both now check readLocalSymlinkTarget() before recursing into a folder and treat it as a single link entry instead, matching how file symlinks are already handled by the existing push/pull machinery. Also fixes createLocalSymlink() to pass the correct `type` hint to Node's symlinkSync on Windows — omitting it defaults to 'file', which produces a broken link when the target is actually a directory (a no-op on macOS/Linux, but required on Windows). Closes #33 --- src/logic/gitignore-manager.ts | 6 ++ src/ui/SyncStatusView.ts | 9 ++ src/utils/symlink.ts | 20 +++- tests/logic/gitignore-manager.test.ts | 22 +++++ tests/setup.ts | 6 ++ tests/utils/symlink.test.ts | 126 ++++++++++++++++++++++++++ 6 files changed, 187 insertions(+), 2 deletions(-) create mode 100644 tests/utils/symlink.test.ts 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(); + }); + }); +});