mirror of
https://github.com/aaronsb/obsidian-mcp-plugin.git
synced 2026-07-22 06:45:14 +00:00
Merge pull request #222 from fa1k3/fix/graph-ignore-exclusion
fix(graph): honor MCP ignore exclusions in graph traversal
This commit is contained in:
commit
bf6e29ae73
8 changed files with 203 additions and 18 deletions
|
|
@ -95,7 +95,15 @@ export class GraphSearchTool {
|
|||
private graphTraversal: GraphTraversal;
|
||||
|
||||
constructor(private api: ObsidianAPI, private app: App) {
|
||||
this.graphTraversal = new GraphTraversal(app);
|
||||
this.graphTraversal = new GraphTraversal(app, api.getIgnoreManager());
|
||||
}
|
||||
|
||||
/** Throw "File not found" for an ignored query root ('/' and '' are the virtual root). */
|
||||
private assertNotExcluded(path?: string): void {
|
||||
if (!path || path === '/') return;
|
||||
if (this.graphTraversal.isExcluded(path)) {
|
||||
throw new Error(`File not found: ${path}`);
|
||||
}
|
||||
}
|
||||
|
||||
/**
|
||||
|
|
@ -103,7 +111,14 @@ export class GraphSearchTool {
|
|||
*/
|
||||
search(params: GraphSearchParams): GraphSearchResult {
|
||||
const { operation } = params;
|
||||
|
||||
|
||||
// Reject excluded query roots so directly targeting an ignored note does
|
||||
// not disclose its relationships. Excluded paths are treated as not found,
|
||||
// matching ObsidianAPI's read-path behavior. The '/' and '' sentinels are
|
||||
// the virtual vault root, not real paths.
|
||||
this.assertNotExcluded(params.sourcePath);
|
||||
this.assertNotExcluded(params.targetPath);
|
||||
|
||||
switch (operation) {
|
||||
case 'traverse':
|
||||
return this.performTraversal(params);
|
||||
|
|
@ -174,7 +189,7 @@ export class GraphSearchTool {
|
|||
edges: result.edges,
|
||||
graphStats: result.stats,
|
||||
message: params.sourcePath === '/' || params.sourcePath === ''
|
||||
? `Traversed from ${Math.min(10, this.app.vault.getFiles().length)} most recent files: Found ${result.stats.totalNodes} connected nodes within ${params.maxDepth} degrees`
|
||||
? `Traversed from ${Math.min(10, this.app.vault.getFiles().filter(f => !this.graphTraversal.isExcluded(f.path)).length)} most recent files: Found ${result.stats.totalNodes} connected nodes within ${params.maxDepth} degrees`
|
||||
: `Found ${result.stats.totalNodes} connected nodes within ${params.maxDepth} degrees of separation`,
|
||||
workflow: {
|
||||
message: 'Graph traversal complete. You can explore individual nodes or find paths between them.',
|
||||
|
|
|
|||
|
|
@ -1,4 +1,5 @@
|
|||
import { App, TFile, CachedMetadata, getAllTags } from 'obsidian';
|
||||
import { MCPIgnoreManager } from '../security/mcp-ignore-manager';
|
||||
|
||||
/**
|
||||
* Represents a node in the Obsidian vault graph
|
||||
|
|
@ -53,7 +54,23 @@ export interface GraphTraversalResult {
|
|||
* Utility class for traversing the Obsidian vault graph
|
||||
*/
|
||||
export class GraphTraversal {
|
||||
constructor(private app: App) {}
|
||||
constructor(private app: App, private ignoreManager?: MCPIgnoreManager) {}
|
||||
|
||||
/**
|
||||
* Whether a path is excluded by the MCP ignore configuration.
|
||||
*
|
||||
* Graph traversal reads directly from metadataCache.resolvedLinks and
|
||||
* vault.getFiles(), both unaware of .mcpignore. Excluded paths must be kept
|
||||
* out of results on two fronts: the link primitives filter the far endpoint
|
||||
* (so an ignored note never appears as a neighbor/edge of a visible query),
|
||||
* and the file enumerations (getAllNodes, tag connections, root traversal,
|
||||
* vault statistics) skip ignored files. The query root itself is rejected at
|
||||
* the GraphSearchTool boundary, so directly querying an ignored note does not
|
||||
* disclose its relationships. Public so that boundary can reuse this check.
|
||||
*/
|
||||
isExcluded(path: string): boolean {
|
||||
return !!this.ignoreManager && this.ignoreManager.isExcluded(path);
|
||||
}
|
||||
|
||||
/**
|
||||
* Get a human-friendly graph node title for a file.
|
||||
|
|
@ -84,7 +101,7 @@ export class GraphTraversal {
|
|||
* Get all nodes (files) in the vault
|
||||
*/
|
||||
getAllNodes(): GraphNode[] {
|
||||
const files = this.app.vault.getFiles();
|
||||
const files = this.app.vault.getFiles().filter(f => !this.isExcluded(f.path));
|
||||
return files.map(file => ({
|
||||
file,
|
||||
path: file.path,
|
||||
|
|
@ -102,6 +119,7 @@ export class GraphTraversal {
|
|||
|
||||
// Search through all files for links to this file
|
||||
for (const sourcePath in resolvedLinks) {
|
||||
if (this.isExcluded(sourcePath)) continue;
|
||||
const links = resolvedLinks[sourcePath];
|
||||
if (links[filePath]) {
|
||||
edges.push({
|
||||
|
|
@ -125,6 +143,7 @@ export class GraphTraversal {
|
|||
|
||||
if (links) {
|
||||
for (const targetPath in links) {
|
||||
if (this.isExcluded(targetPath)) continue;
|
||||
edges.push({
|
||||
source: filePath,
|
||||
target: targetPath,
|
||||
|
|
@ -167,7 +186,7 @@ export class GraphTraversal {
|
|||
const edges: GraphEdge[] = [];
|
||||
const file = this.app.vault.getAbstractFileByPath(filePath);
|
||||
|
||||
if (!(file instanceof TFile)) return edges;
|
||||
if (!(file instanceof TFile) || this.isExcluded(filePath)) return edges;
|
||||
|
||||
const cache = this.app.metadataCache.getFileCache(file);
|
||||
const fileTags = new Set(cache ? getAllTags(cache) || [] : []);
|
||||
|
|
@ -177,6 +196,7 @@ export class GraphTraversal {
|
|||
const files = this.app.vault.getFiles();
|
||||
for (const otherFile of files) {
|
||||
if (otherFile.path === filePath) continue;
|
||||
if (this.isExcluded(otherFile.path)) continue;
|
||||
|
||||
const otherCache = this.app.metadataCache.getFileCache(otherFile);
|
||||
const otherTags = otherCache ? getAllTags(otherCache) || [] : [];
|
||||
|
|
@ -224,7 +244,9 @@ export class GraphTraversal {
|
|||
const allFiles = this.app.vault.getFiles();
|
||||
|
||||
// Sort by modification time to get most relevant files first
|
||||
const sortedFiles = allFiles.sort((a, b) => b.stat.mtime - a.stat.mtime);
|
||||
const sortedFiles = allFiles
|
||||
.filter(file => !this.isExcluded(file.path))
|
||||
.sort((a, b) => b.stat.mtime - a.stat.mtime);
|
||||
|
||||
// Take up to 10 most recently modified files as starting points
|
||||
const startingFiles = sortedFiles.slice(0, Math.min(10, sortedFiles.length));
|
||||
|
|
@ -256,7 +278,8 @@ export class GraphTraversal {
|
|||
const { path, depth } = queue.shift()!;
|
||||
|
||||
if (visited.has(path) || depth > maxDepth) continue;
|
||||
|
||||
if (this.isExcluded(path)) continue;
|
||||
|
||||
const file = this.app.vault.getAbstractFileByPath(path);
|
||||
if (!(file instanceof TFile)) continue;
|
||||
|
||||
|
|
@ -411,7 +434,7 @@ export class GraphTraversal {
|
|||
// Handle root path "/" specially
|
||||
if (filePath === '/' || filePath === '') {
|
||||
// For root, return a virtual node representing the vault with recent files as neighbors
|
||||
const allFiles = this.app.vault.getFiles();
|
||||
const allFiles = this.app.vault.getFiles().filter(f => !this.isExcluded(f.path));
|
||||
const sortedFiles = allFiles.sort((a, b) => b.stat.mtime - a.stat.mtime);
|
||||
const recentFiles = sortedFiles.slice(0, Math.min(20, sortedFiles.length));
|
||||
|
||||
|
|
@ -502,7 +525,7 @@ export class GraphTraversal {
|
|||
largestComponentSize: number;
|
||||
isolatedClusters: number;
|
||||
} {
|
||||
const mdFiles = this.app.vault.getFiles().filter(f => f.extension === 'md');
|
||||
const mdFiles = this.app.vault.getFiles().filter(f => f.extension === 'md' && !this.isExcluded(f.path));
|
||||
const totalNotes = mdFiles.length;
|
||||
|
||||
const resolved = this.app.metadataCache.resolvedLinks ?? {};
|
||||
|
|
|
|||
|
|
@ -100,6 +100,11 @@ export class ObsidianAPI {
|
|||
return this.app;
|
||||
}
|
||||
|
||||
// Getter to access the ignore manager so graph traversal can honor exclusions
|
||||
getIgnoreManager(): MCPIgnoreManager | undefined {
|
||||
return this.ignoreManager;
|
||||
}
|
||||
|
||||
// Server info
|
||||
getServerInfo() {
|
||||
const baseInfo = {
|
||||
|
|
|
|||
142
tests/graph-ignore-exclusion.test.ts
Normal file
142
tests/graph-ignore-exclusion.test.ts
Normal file
|
|
@ -0,0 +1,142 @@
|
|||
import { App, TFile } from 'obsidian';
|
||||
import { GraphTraversal } from '../src/utils/graph-traversal';
|
||||
import { GraphSearchTool } from '../src/tools/graph-search';
|
||||
import { ObsidianAPI } from '../src/utils/obsidian-api';
|
||||
import { MCPIgnoreManager } from '../src/security/mcp-ignore-manager';
|
||||
|
||||
function makeFile(path: string): TFile {
|
||||
const file = new TFile();
|
||||
file.path = path;
|
||||
file.name = path.split('/').pop() ?? path;
|
||||
file.basename = file.name.replace(/\.md$/, '');
|
||||
file.extension = 'md';
|
||||
(file as any).stat = { mtime: 0, ctime: 0, size: 0 };
|
||||
return file;
|
||||
}
|
||||
|
||||
// Fake ignore manager that excludes anything under _build/backups/
|
||||
const ignoreManager = {
|
||||
isExcluded: (path: string) => path.startsWith('_build/backups/')
|
||||
} as unknown as MCPIgnoreManager;
|
||||
|
||||
const FILES: Record<string, TFile> = {
|
||||
'stack/index.md': makeFile('stack/index.md'),
|
||||
'real.md': makeFile('real.md'),
|
||||
'_build/backups/snapshot.md': makeFile('_build/backups/snapshot.md'),
|
||||
'_build/backups/old.md': makeFile('_build/backups/old.md')
|
||||
};
|
||||
|
||||
// Per-path tag cache — all four notes share #archived
|
||||
const TAGS: Record<string, string> = {
|
||||
'stack/index.md': '#archived',
|
||||
'real.md': '#archived',
|
||||
'_build/backups/snapshot.md': '#archived',
|
||||
'_build/backups/old.md': '#archived'
|
||||
};
|
||||
|
||||
function makeApp(): App {
|
||||
const app = new App();
|
||||
(app as any).metadataCache = {
|
||||
resolvedLinks: {
|
||||
// stack/index.md links out to a real note AND a backup note
|
||||
'stack/index.md': { 'real.md': 1, '_build/backups/old.md': 1 },
|
||||
// a backup snapshot links back into stack/index.md
|
||||
'_build/backups/snapshot.md': { 'stack/index.md': 1 },
|
||||
'real.md': { 'stack/index.md': 1 }
|
||||
},
|
||||
unresolvedLinks: {},
|
||||
getFileCache: jest.fn((file: any) => ({
|
||||
tags: TAGS[file?.path] ? [{ tag: TAGS[file.path] }] : []
|
||||
}))
|
||||
};
|
||||
app.vault.getAbstractFileByPath = jest.fn((path: string) => FILES[path] ?? null);
|
||||
app.vault.getFiles = jest.fn(() => Object.values(FILES));
|
||||
return app;
|
||||
}
|
||||
|
||||
/**
|
||||
* Graph traversal reads metadataCache.resolvedLinks and vault.getFiles()
|
||||
* directly, both unaware of .mcpignore. These tests pin that ignored paths are
|
||||
* kept out of (a) the link primitives, (b) the file enumerations (root, tags,
|
||||
* vault stats, all-nodes), and (c) are rejected as query roots at the
|
||||
* GraphSearchTool boundary so a direct query can't disclose their relationships.
|
||||
*/
|
||||
describe('GraphTraversal — link primitives & enumerations honor exclusions', () => {
|
||||
let app: App;
|
||||
beforeEach(() => { app = makeApp(); });
|
||||
|
||||
it('filters excluded sources out of backlinks', () => {
|
||||
const sources = new GraphTraversal(app, ignoreManager).getBacklinks('stack/index.md').map(e => e.source);
|
||||
expect(sources).toContain('real.md');
|
||||
expect(sources).not.toContain('_build/backups/snapshot.md');
|
||||
});
|
||||
|
||||
it('filters excluded targets out of forward links', () => {
|
||||
const targets = new GraphTraversal(app, ignoreManager).getForwardLinks('stack/index.md').map(e => e.target);
|
||||
expect(targets).toContain('real.md');
|
||||
expect(targets).not.toContain('_build/backups/old.md');
|
||||
});
|
||||
|
||||
it('keeps excluded nodes out of the local neighborhood', () => {
|
||||
const { neighbors, edges } = new GraphTraversal(app, ignoreManager).getLocalNeighborhood('stack/index.md');
|
||||
const paths = neighbors.map(n => n.path);
|
||||
expect(paths).toContain('real.md');
|
||||
expect(paths).not.toContain('_build/backups/old.md');
|
||||
expect(paths).not.toContain('_build/backups/snapshot.md');
|
||||
expect(edges.every(e => !e.source.startsWith('_build/backups/') && !e.target.startsWith('_build/backups/'))).toBe(true);
|
||||
});
|
||||
|
||||
it('excludes ignored files from the root neighborhood', () => {
|
||||
const { neighbors } = new GraphTraversal(app, ignoreManager).getLocalNeighborhood('/');
|
||||
expect(neighbors.map(n => n.path).some(p => p.startsWith('_build/backups/'))).toBe(false);
|
||||
});
|
||||
|
||||
it('excludes ignored files from getAllNodes', () => {
|
||||
const paths = new GraphTraversal(app, ignoreManager).getAllNodes().map(n => n.path);
|
||||
expect(paths).toContain('stack/index.md');
|
||||
expect(paths.some(p => p.startsWith('_build/backups/'))).toBe(false);
|
||||
});
|
||||
|
||||
it('excludes ignored files from tag connections', () => {
|
||||
const targets = new GraphTraversal(app, ignoreManager).getTagConnections('stack/index.md').map(e => e.target);
|
||||
expect(targets).toContain('real.md');
|
||||
expect(targets.some(p => p.startsWith('_build/backups/'))).toBe(false);
|
||||
});
|
||||
|
||||
it('excludes ignored files from vault statistics totalNotes', () => {
|
||||
// 4 files total, 2 under _build/backups/ → only 2 visible notes
|
||||
expect(new GraphTraversal(app, ignoreManager).getVaultStatistics().totalNotes).toBe(2);
|
||||
});
|
||||
|
||||
it('leaves everything unfiltered when no ignore manager is configured', () => {
|
||||
const t = new GraphTraversal(app);
|
||||
expect(t.getForwardLinks('stack/index.md').map(e => e.target)).toContain('_build/backups/old.md');
|
||||
expect(t.getBacklinks('stack/index.md').map(e => e.source)).toContain('_build/backups/snapshot.md');
|
||||
expect(t.getVaultStatistics().totalNotes).toBe(4);
|
||||
});
|
||||
});
|
||||
|
||||
describe('GraphSearchTool — rejects excluded query roots (no relationship disclosure)', () => {
|
||||
let tool: GraphSearchTool;
|
||||
beforeEach(() => {
|
||||
const app = makeApp();
|
||||
tool = new GraphSearchTool({ getIgnoreManager: () => ignoreManager } as unknown as ObsidianAPI, app);
|
||||
});
|
||||
|
||||
for (const operation of ['backlinks', 'forwardlinks', 'neighbors', 'statistics'] as const) {
|
||||
it(`throws "File not found" for ${operation} on an excluded sourcePath`, () => {
|
||||
expect(() => tool.search({ operation, sourcePath: '_build/backups/snapshot.md' }))
|
||||
.toThrow(/File not found/);
|
||||
});
|
||||
}
|
||||
|
||||
it('throws "File not found" for path when targetPath is excluded', () => {
|
||||
expect(() => tool.search({ operation: 'path', sourcePath: 'real.md', targetPath: '_build/backups/old.md' }))
|
||||
.toThrow(/File not found/);
|
||||
});
|
||||
|
||||
it('does not reject the "/" vault root, and the root excludes ignored neighbors', () => {
|
||||
const result = tool.search({ operation: 'neighbors', sourcePath: '/' });
|
||||
expect((result.nodes ?? []).map(n => n.path).some(p => p.startsWith('_build/backups/'))).toBe(false);
|
||||
});
|
||||
});
|
||||
|
|
@ -47,7 +47,7 @@ describe('graph node titles', () => {
|
|||
app.vault.getAbstractFileByPath = jest.fn((path: string) => filesByPath.get(path) ?? null);
|
||||
|
||||
traversal = new GraphTraversal(app);
|
||||
search = new GraphSearchTool({} as ObsidianAPI, app);
|
||||
search = new GraphSearchTool({ getIgnoreManager: () => undefined } as unknown as ObsidianAPI, app);
|
||||
});
|
||||
|
||||
it('uses the parent folder as the graph title for index files', () => {
|
||||
|
|
|
|||
|
|
@ -17,7 +17,7 @@ const mockApp = {
|
|||
}
|
||||
} as unknown as App;
|
||||
|
||||
const mockAPI = {} as ObsidianAPI;
|
||||
const mockAPI = { getIgnoreManager: () => undefined } as unknown as ObsidianAPI;
|
||||
const mockSearchCore = new SearchCore(mockApp);
|
||||
|
||||
describe('GraphSearchTraversal', () => {
|
||||
|
|
|
|||
|
|
@ -34,7 +34,7 @@ describe('GraphSearchTool', () => {
|
|||
path === 'resolved.md' ? resolvedTarget : null
|
||||
);
|
||||
|
||||
tool = new GraphSearchTool({} as ObsidianAPI, app);
|
||||
tool = new GraphSearchTool({ getIgnoreManager: () => undefined } as unknown as ObsidianAPI, app);
|
||||
});
|
||||
|
||||
it('omits unresolved forward links by default', () => {
|
||||
|
|
|
|||
|
|
@ -58,7 +58,7 @@ describe('graph.statistics — vault-wide (#132)', () => {
|
|||
'd.md': { 'e.md': 1 },
|
||||
},
|
||||
});
|
||||
tool = new GraphSearchTool({} as ObsidianAPI, app);
|
||||
tool = new GraphSearchTool({ getIgnoreManager: () => undefined } as unknown as ObsidianAPI, app);
|
||||
});
|
||||
|
||||
it('returns vaultStatistics when sourcePath is omitted', () => {
|
||||
|
|
@ -95,7 +95,7 @@ describe('graph.statistics — vault-wide (#132)', () => {
|
|||
files: [A, B],
|
||||
resolvedLinks: { 'a.md': { 'b.md': 3 } },
|
||||
});
|
||||
const t = new GraphSearchTool({} as ObsidianAPI, app);
|
||||
const t = new GraphSearchTool({ getIgnoreManager: () => undefined } as unknown as ObsidianAPI, app);
|
||||
|
||||
const result = t.search({ operation: 'statistics' });
|
||||
expect(result.vaultStatistics).toMatchObject({
|
||||
|
|
@ -108,7 +108,7 @@ describe('graph.statistics — vault-wide (#132)', () => {
|
|||
|
||||
it('handles an empty vault without dividing by zero', () => {
|
||||
const app = buildApp({ files: [], resolvedLinks: {} });
|
||||
const t = new GraphSearchTool({} as ObsidianAPI, app);
|
||||
const t = new GraphSearchTool({ getIgnoreManager: () => undefined } as unknown as ObsidianAPI, app);
|
||||
|
||||
const result = t.search({ operation: 'statistics' });
|
||||
expect(result.vaultStatistics).toEqual({
|
||||
|
|
@ -127,7 +127,7 @@ describe('graph.statistics — vault-wide (#132)', () => {
|
|||
files: [A, B, image],
|
||||
resolvedLinks: { 'a.md': { 'b.md': 1 } },
|
||||
});
|
||||
const t = new GraphSearchTool({} as ObsidianAPI, app);
|
||||
const t = new GraphSearchTool({ getIgnoreManager: () => undefined } as unknown as ObsidianAPI, app);
|
||||
|
||||
const result = t.search({ operation: 'statistics' });
|
||||
expect(result.vaultStatistics?.totalNotes).toBe(2);
|
||||
|
|
@ -142,7 +142,7 @@ describe('graph.statistics — vault-wide (#132)', () => {
|
|||
'b.md': { 'a.md': 1 },
|
||||
},
|
||||
});
|
||||
const t = new GraphSearchTool({} as ObsidianAPI, app);
|
||||
const t = new GraphSearchTool({ getIgnoreManager: () => undefined } as unknown as ObsidianAPI, app);
|
||||
|
||||
const result = t.search({ operation: 'statistics' });
|
||||
expect(result.vaultStatistics).toMatchObject({
|
||||
|
|
|
|||
Loading…
Reference in a new issue