mirror of
https://github.com/aaronsb/obsidian-mcp-plugin.git
synced 2026-07-22 06:45:14 +00:00
Merge pull request #158 from aaronsb/feat/vault-list-pagination-hint
feat(vault.list): Coherent pagination + agent next-call hints
This commit is contained in:
commit
ebb7f7faec
6 changed files with 163 additions and 14 deletions
|
|
@ -283,6 +283,28 @@ function normalizeResponse(key: string, response: unknown): NormalizedResponse {
|
|||
return resp;
|
||||
}
|
||||
|
||||
// vault.list (paginated): router returns
|
||||
// {files: [{path, name, type: 'file'|'folder', ...}], page, pageSize,
|
||||
// totalFiles, totalPages, directory}
|
||||
// Formatter expects FileListResponse with isFolder boolean. Translate
|
||||
// shape so the structured branch renders correctly (previously it
|
||||
// looked for f.isFolder which was always undefined, lumping every
|
||||
// entry into "files" regardless of actual type).
|
||||
case 'vault.list': {
|
||||
const listResp = resp as { files?: unknown };
|
||||
if (Array.isArray(listResp.files)) {
|
||||
const items = listResp.files as Array<Record<string, unknown>>;
|
||||
return {
|
||||
...resp,
|
||||
files: items.map(item => ({
|
||||
...item,
|
||||
isFolder: item.type === 'folder',
|
||||
})),
|
||||
};
|
||||
}
|
||||
return resp;
|
||||
}
|
||||
|
||||
// vault.fragments: router returns {result: [...fragments across files]}
|
||||
// Transform to grouped format for formatter
|
||||
case 'vault.fragments': {
|
||||
|
|
|
|||
|
|
@ -30,6 +30,10 @@ export interface FileListResponse {
|
|||
files: FileListItem[];
|
||||
totalFiles?: number;
|
||||
totalFolders?: number;
|
||||
// Pagination metadata when listFilesPaginated is the underlying call.
|
||||
page?: number;
|
||||
pageSize?: number;
|
||||
totalPages?: number;
|
||||
}
|
||||
|
||||
export function formatFileList(response: FileListResponse | string[]): string {
|
||||
|
|
@ -38,19 +42,24 @@ export function formatFileList(response: FileListResponse | string[]): string {
|
|||
// Handle simple string array response
|
||||
if (Array.isArray(response)) {
|
||||
const paths = response;
|
||||
const truncateAt = 50;
|
||||
lines.push(header(1, 'Files'));
|
||||
lines.push('');
|
||||
lines.push(`Found ${paths.length} item${paths.length !== 1 ? 's' : ''}`);
|
||||
lines.push('');
|
||||
|
||||
paths.slice(0, 50).forEach(path => {
|
||||
paths.slice(0, truncateAt).forEach(path => {
|
||||
const name = path.split('/').pop() || path;
|
||||
const isFolder = !path.includes('.');
|
||||
lines.push(`- ${isFolder ? name + '/' : name}`);
|
||||
});
|
||||
|
||||
if (paths.length > 50) {
|
||||
lines.push(`- ... and ${paths.length - 50} more`);
|
||||
if (paths.length > truncateAt) {
|
||||
lines.push(`- ... and ${paths.length - truncateAt} more`);
|
||||
lines.push('');
|
||||
// Surface a concrete next call so an agent that needs items past
|
||||
// the truncation point doesn't have to guess.
|
||||
lines.push(tip(`Showing first ${truncateAt} of ${paths.length}. Call again with \`page=2 pageSize=${truncateAt}\` to continue, or \`raw: true\` to receive the full list in one response.`));
|
||||
}
|
||||
|
||||
lines.push('');
|
||||
|
|
@ -61,7 +70,7 @@ export function formatFileList(response: FileListResponse | string[]): string {
|
|||
}
|
||||
|
||||
// Handle structured response
|
||||
const { directory, files, totalFiles, totalFolders } = response;
|
||||
const { directory, files, totalFiles, totalFolders, page, pageSize, totalPages } = response;
|
||||
|
||||
lines.push(header(1, `Directory: ${directory || '/'}`));
|
||||
lines.push('');
|
||||
|
|
@ -69,7 +78,8 @@ export function formatFileList(response: FileListResponse | string[]): string {
|
|||
const folders = files.filter(f => f.isFolder);
|
||||
const regularFiles = files.filter(f => !f.isFolder);
|
||||
|
||||
// Summary
|
||||
// Summary — show pagination state when present so the agent sees what
|
||||
// slice of the universe it's looking at.
|
||||
const summaryParts: string[] = [];
|
||||
if (totalFolders !== undefined || folders.length > 0) {
|
||||
summaryParts.push(`${totalFolders ?? folders.length} folders`);
|
||||
|
|
@ -81,6 +91,10 @@ export function formatFileList(response: FileListResponse | string[]): string {
|
|||
lines.push(summaryParts.join(', '));
|
||||
lines.push('');
|
||||
}
|
||||
if (page !== undefined && totalPages !== undefined && totalPages > 0) {
|
||||
lines.push(`Page ${page} of ${totalPages}${pageSize ? ` (${pageSize} per page)` : ''}`);
|
||||
lines.push('');
|
||||
}
|
||||
|
||||
// Folders first
|
||||
if (folders.length > 0) {
|
||||
|
|
@ -107,6 +121,15 @@ export function formatFileList(response: FileListResponse | string[]): string {
|
|||
lines.push('');
|
||||
}
|
||||
|
||||
// Concrete next-call hint when there are more pages — agent doesn't
|
||||
// have to guess at the next move.
|
||||
if (page !== undefined && totalPages !== undefined && page < totalPages) {
|
||||
const dirArg = directory ? `directory='${directory}', ` : '';
|
||||
const sizeArg = pageSize ? `, pageSize=${pageSize}` : '';
|
||||
lines.push(tip(`More results available. Call \`vault.list(${dirArg}page=${page + 1}${sizeArg})\` for the next page.`));
|
||||
lines.push('');
|
||||
}
|
||||
|
||||
lines.push(divider());
|
||||
lines.push(tip('Use `vault.read(path)` to read a file, or `vault.list(directory)` to explore a folder'));
|
||||
lines.push(summaryFooter());
|
||||
|
|
|
|||
|
|
@ -66,16 +66,16 @@ export class SecureObsidianAPI extends ObsidianAPI {
|
|||
return super.listFiles(listPath);
|
||||
}
|
||||
|
||||
async listFilesPaginated(directory?: string, page: number = 1, pageSize: number = 20): ReturnType<ObsidianAPI['listFilesPaginated']> {
|
||||
async listFilesPaginated(directory?: string, page: number = 1, pageSize: number = 20, recursive: boolean = false): ReturnType<ObsidianAPI['listFilesPaginated']> {
|
||||
const validated = await this.security.validateOperation({
|
||||
type: OperationType.READ,
|
||||
path: directory || '.',
|
||||
context: { method: 'listFilesPaginated', page, pageSize }
|
||||
context: { method: 'listFilesPaginated', page, pageSize, recursive }
|
||||
});
|
||||
|
||||
// Use validated path if directory was provided, undefined for vault root
|
||||
const listPath = !validated.path || validated.path === '.' ? undefined : validated.path;
|
||||
return super.listFilesPaginated(listPath, page, pageSize);
|
||||
return super.listFilesPaginated(listPath, page, pageSize, recursive);
|
||||
}
|
||||
|
||||
async getActiveFile(): Promise<ObsidianFile> {
|
||||
|
|
|
|||
|
|
@ -162,11 +162,19 @@ export class SemanticRouter {
|
|||
const dirParam = paramStr(params, 'directory');
|
||||
const directory = dirParam === '/' ? undefined : dirParam;
|
||||
|
||||
// Use paginated list if page parameters are provided
|
||||
// Use paginated list if page parameters are provided.
|
||||
// When paginating inside a specific directory, recurse so the
|
||||
// agent sees the same universe of files as the non-paginated
|
||||
// call — page N gives the Nth slice of the recursive listing,
|
||||
// not a level-only folder enumeration. The root case is
|
||||
// already recursive (getAllLoadedFiles), so we leave
|
||||
// recursive=false there; listFilesPaginated routes through
|
||||
// the same path regardless.
|
||||
if (params.page || params.pageSize) {
|
||||
const page = parseInt(paramStr(params, 'page') ?? '1') || 1;
|
||||
const pageSize = parseInt(paramStr(params, 'pageSize') ?? '20') || 20;
|
||||
return await this.api.listFilesPaginated(directory, page, pageSize);
|
||||
const recursive = directory !== undefined;
|
||||
return await this.api.listFilesPaginated(directory, page, pageSize, recursive);
|
||||
}
|
||||
|
||||
// Fallback to simple list for backwards compatibility
|
||||
|
|
|
|||
|
|
@ -265,7 +265,8 @@ export class ObsidianAPI {
|
|||
listFilesPaginated(
|
||||
directory?: string,
|
||||
page: number = 1,
|
||||
pageSize: number = 20
|
||||
pageSize: number = 20,
|
||||
recursive: boolean = false
|
||||
): Promise<{
|
||||
files: Array<{
|
||||
path: string;
|
||||
|
|
@ -283,13 +284,29 @@ export class ObsidianAPI {
|
|||
}> {
|
||||
const vault = this.app.vault;
|
||||
let files: TAbstractFile[];
|
||||
|
||||
|
||||
if (directory && directory !== '/') {
|
||||
const folder = vault.getAbstractFileByPath(directory);
|
||||
if (!folder || !(folder instanceof TFolder)) {
|
||||
throw new Error(`Directory not found: ${directory}`);
|
||||
}
|
||||
files = folder.children;
|
||||
if (recursive) {
|
||||
// Walk the tree and keep only files — pagination is over the same
|
||||
// flat universe listFiles() returns, so an agent paging through a
|
||||
// directory sees a coherent, consistent set of items.
|
||||
files = [];
|
||||
const stack: TAbstractFile[] = [...folder.children];
|
||||
while (stack.length > 0) {
|
||||
const f = stack.pop()!;
|
||||
if (f instanceof TFile) {
|
||||
files.push(f);
|
||||
} else if (f instanceof TFolder) {
|
||||
stack.push(...f.children);
|
||||
}
|
||||
}
|
||||
} else {
|
||||
files = folder.children;
|
||||
}
|
||||
} else {
|
||||
files = vault.getAllLoadedFiles();
|
||||
}
|
||||
|
|
@ -311,7 +328,16 @@ export class ObsidianAPI {
|
|||
|
||||
return result;
|
||||
}).sort((a: FileDetailObject, b: FileDetailObject) => {
|
||||
// Sort folders first, then files, alphabetically
|
||||
if (recursive) {
|
||||
// Match listFiles() — full-path lexicographic. The agent's
|
||||
// "use page=N" hint anchors to the same total order, so page
|
||||
// boundaries are stable across calls and basename collisions
|
||||
// (e.g. two index.md in different subfolders) sort deterministically.
|
||||
return a.path.localeCompare(b.path);
|
||||
}
|
||||
// Level-only mode: folders first, then files by basename — what
|
||||
// existing internal callers (cp recursion, isDirectory probe)
|
||||
// depend on for tree navigation.
|
||||
if (a.type !== b.type) {
|
||||
return a.type === 'folder' ? -1 : 1;
|
||||
}
|
||||
|
|
|
|||
|
|
@ -90,3 +90,73 @@ describe('ObsidianAPI.listFiles — folder-of-folders (#154)', () => {
|
|||
]);
|
||||
});
|
||||
});
|
||||
|
||||
describe('ObsidianAPI.listFilesPaginated — recursive mode', () => {
|
||||
let api: ObsidianAPI;
|
||||
let mockApp: App;
|
||||
let lookup: Map<string, TFile | TFolder>;
|
||||
|
||||
beforeEach(() => {
|
||||
// 5 files spread across nested subfolders so a page boundary lands mid-tree.
|
||||
const files = [
|
||||
makeFile('docs/a/one.md'),
|
||||
makeFile('docs/a/two.md'),
|
||||
makeFile('docs/b/three.md'),
|
||||
makeFile('docs/c/sub/four.md'),
|
||||
makeFile('docs/c/sub/five.md'),
|
||||
];
|
||||
const a = makeFolder('docs/a', [files[0], files[1]]);
|
||||
const b = makeFolder('docs/b', [files[2]]);
|
||||
const sub = makeFolder('docs/c/sub', [files[3], files[4]]);
|
||||
const c = makeFolder('docs/c', [sub]);
|
||||
const docs = makeFolder('docs', [a, b, c]);
|
||||
|
||||
const all = [docs, a, b, c, sub, ...files];
|
||||
lookup = new Map(all.map(f => [f.path, f]));
|
||||
|
||||
mockApp = new App();
|
||||
mockApp.vault.getAbstractFileByPath = (path: string) => lookup.get(path) ?? null;
|
||||
mockApp.vault.getAllLoadedFiles = () => all;
|
||||
mockApp.vault.adapter = { basePath: '/mock' } as any;
|
||||
|
||||
api = new ObsidianAPI(mockApp);
|
||||
});
|
||||
|
||||
it('paginates over the recursive file set when recursive=true', async () => {
|
||||
const page1 = await api.listFilesPaginated('docs', 1, 2, true);
|
||||
expect(page1.totalFiles).toBe(5);
|
||||
expect(page1.totalPages).toBe(3);
|
||||
expect(page1.files).toHaveLength(2);
|
||||
// Every entry should be a TFile — folders are filtered out in recursive mode.
|
||||
expect(page1.files.every(f => f.type === 'file')).toBe(true);
|
||||
|
||||
const page2 = await api.listFilesPaginated('docs', 2, 2, true);
|
||||
expect(page2.files).toHaveLength(2);
|
||||
// No overlap between consecutive pages.
|
||||
const seen = new Set(page1.files.map(f => f.path));
|
||||
expect(page2.files.every(f => !seen.has(f.path))).toBe(true);
|
||||
});
|
||||
|
||||
it('paged universe matches the flat listFiles universe (agent contract)', async () => {
|
||||
// The Settings UI / formatter tells the agent "use page=2 pageSize=N to
|
||||
// continue". That hint only works if concatenating pages reproduces the
|
||||
// same total order that the non-paginated listFiles call returned.
|
||||
const flat = await api.listFiles('docs');
|
||||
|
||||
const pageSize = 2;
|
||||
const collected: string[] = [];
|
||||
const total = (await api.listFilesPaginated('docs', 1, pageSize, true)).totalPages;
|
||||
for (let page = 1; page <= total; page++) {
|
||||
const slice = await api.listFilesPaginated('docs', page, pageSize, true);
|
||||
collected.push(...slice.files.map(f => f.path));
|
||||
}
|
||||
expect(collected).toEqual(flat);
|
||||
});
|
||||
|
||||
it('preserves level-only behavior when recursive=false (default)', async () => {
|
||||
const result = await api.listFilesPaginated('docs', 1, 20, false);
|
||||
// docs/ has three subfolders directly; should see those, not the leaf files.
|
||||
expect(result.totalFiles).toBe(3);
|
||||
expect(result.files.every(f => f.type === 'folder')).toBe(true);
|
||||
});
|
||||
});
|
||||
|
|
|
|||
Loading…
Reference in a new issue