fix: bound what the Files panel walk reads and keep its subfolders in it (release review)

The round-robin walk capped what it listed, not what it read: every queued
folder was read whole, sorted and held until drained, so one GET could pull
millions of entries (100 folders of 50,000 files killed a 400 MB heap). A read
now keeps only the entries the listing can use, and MAX_WALK_ENTRIES_READ stops
opening folders at 250,000 entries, leaving the rest queued and flagged.

A folder also spent its whole share on its own files, leaving nothing for the
subfolders it had just found, alone in its round or beside a sibling, so search
hits inside them were lost. Its files now count as one more share beside those
subfolders. Math.max(directoriesInRound, 2) would have fixed only the sole-folder
case and cut a wide root's folder list short.

Folders below the walk root are probed before their readdir (#516), since the
walk reaches folders the depth-first one never did.

Filtering at the read also clears the false flag on a folder whose only unread
entries were hidden or excluded, and an exactly-full workspace now finishes.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
This commit is contained in:
Codeman maintainer committed 2026-11-06 03:53:08 +01:00
1 parent a908be05ac
commit fafec35344
8 files changed
+379 -66

No files matched your search

+95 -5
View File
@@ -172,8 +172,8 @@ describe('files endpoint search mode', () => {
}
});
it('copes with the root itself being unfinished', async () => {
// The root has no node to flag. Its folders are listed before its files, and none is read.
it('copes with the root itself being unfinished, and its subfolders still get read', async () => {
// The root has no node to flag. Its files stop the walk, but only after the folder it found.
const wide = mkdtempSync(join(tmpdir(), 'codeman-file-root-'));
mkdirSync(join(wide, 'first'));
writeFileSync(join(wide, 'first', 'inside.txt'), '');
@@ -185,9 +185,99 @@ describe('files endpoint search mode', () => {
try {
const { data } = await get('');
expect(data.truncated).toBe(true);
expect(data.tree).toHaveLength(5000);
expect(data.tree[0]).toMatchObject({ name: 'first', incomplete: true, children: [] });
expect(data.tree.filter((node: { incomplete?: boolean }) => node.incomplete)).toHaveLength(1);
expect(data.totalFiles + data.totalDirectories).toBe(5000);
expect(data.tree[0]).toMatchObject({ name: 'first', children: [{ name: 'inside.txt' }] });
expect(data.tree[0]).not.toHaveProperty('incomplete');
const search = await get('?q=inside');
expect(search.data.matches.map((match: { path: string }) => match.path)).toEqual([join('first', 'inside.txt')]);
expect(search.data.truncated).toBe(true);
} finally {
rmSync(wide, { recursive: true, force: true });
}
});
it('does not let a large folder alone in its round starve the subfolders it holds', async () => {
// One level below the walk root, with a second level of subfolder to reach.
const wide = mkdtempSync(join(tmpdir(), 'codeman-file-sole-'));
mkdirSync(join(wide, 'repo', 'src', 'lib'), { recursive: true });
writeFileSync(join(wide, 'repo', 'src', 'lib', 'needle.ts'), '');
for (let index = 0; index < 5100; index++) {
writeFileSync(join(wide, 'repo', `item-${index}.txt`), '');
}
harness.ctx._session.workingDir = wide;
try {
const { data } = await get('?q=needle');
expect(data.matches.map((match: { path: string }) => match.path)).toEqual([
join('repo', 'src', 'lib', 'needle.ts'),
]);
expect(data.truncated).toBe(true);
} finally {
rmSync(wide, { recursive: true, force: true });
}
});
it('does not let large sibling folders starve the subfolders they hold', async () => {
// Two in a round, so "alone in its round" is not what makes this go wrong: each folder's
// equal share used to be spent on its own files, leaving nothing for what it had just found.
const wide = mkdtempSync(join(tmpdir(), 'codeman-file-pair-'));
for (const name of ['a-big', 'b-big']) {
mkdirSync(join(wide, name, 'src'), { recursive: true });
writeFileSync(join(wide, name, 'src', 'needle.ts'), '');
for (let index = 0; index < 5100; index++) {
writeFileSync(join(wide, name, `item-${index}.txt`), '');
}
}
harness.ctx._session.workingDir = wide;
try {
const { data } = await get('?q=needle');
expect(data.matches.map((match: { path: string }) => match.path).sort()).toEqual([
join('a-big', 'src', 'needle.ts'),
join('b-big', 'src', 'needle.ts'),
]);
} finally {
rmSync(wide, { recursive: true, force: true });
}
});
it('calls a workspace of exactly 5,000 nodes finished', async () => {
const exact = mkdtempSync(join(tmpdir(), 'codeman-file-exact-'));
mkdirSync(join(exact, 'empty'));
for (let index = 0; index < 4999; index++) {
writeFileSync(join(exact, `item-${index}.txt`), '');
}
harness.ctx._session.workingDir = exact;
try {
const { data } = await get('');
expect(data.totalFiles + data.totalDirectories).toBe(5000);
expect(data.truncated).toBe(false);
expect(JSON.stringify(data)).not.toContain('incomplete');
} finally {
rmSync(exact, { recursive: true, force: true });
}
});
it('does not flag a folder whose only unread entries are ones the listing leaves out', async () => {
// 3,000 folders leave 2,000 of the budget, which buys each of the first 2,000 its `src`. All
// that is left unread in them is `vendor` and `.hidden`, which would never be listed anyway.
const wide = mkdtempSync(join(tmpdir(), 'codeman-file-excluded-'));
for (let index = 0; index < 3000; index++) {
const dir = join(wide, `dir-${String(index).padStart(4, '0')}`);
mkdirSync(join(dir, 'src'), { recursive: true });
mkdirSync(join(dir, 'vendor'));
writeFileSync(join(dir, '.hidden'), '');
}
harness.ctx._session.workingDir = wide;
try {
const { data } = await get('');
expect(data.truncated).toBe(true);
expect(data.tree[0].children.map((node: { name: string }) => node.name)).toEqual(['src']);
expect(data.tree[0]).not.toHaveProperty('incomplete');
expect(data.tree[2999]).toMatchObject({ incomplete: true, children: [] });
} finally {
rmSync(wide, { recursive: true, force: true });
}
+160
View File
@@ -0,0 +1,160 @@
/**
* GET /api/sessions/:id/files: what one listing costs the server.
*
* test/routes/file-search-mode.test.ts pins what the walk returns, on real folders. This file pins
* what it reads, which is where a real tree cannot go: 100 folders of 50,000 files is minutes of
* setup, and a dead network mount cannot be made in a test. So `fs.readdir` and `fs.stat` are
* stubbed here, on the one object the route and the bounded probe both call through.
*
* Port: none (app.inject).
*/
import { Dirent, mkdirSync, mkdtempSync, rmSync, writeFileSync } from 'node:fs';
import fs from 'node:fs/promises';
import { tmpdir } from 'node:os';
import { join } from 'node:path';
import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest';
import { createRouteTestHarness, type RouteTestHarness } from './_route-test-utils.js';
import { registerFileRoutes } from '../../src/web/routes/file-routes.js';
const UV_DIRENT_FILE = 1;
const UV_DIRENT_DIR = 2;
const dirent = (name: string, type: number, parent: string): Dirent =>
new (Dirent as unknown as new (name: string, type: number, path: string) => Dirent)(name, type, parent);
describe('files endpoint read bounds', () => {
let harness: RouteTestHarness;
beforeEach(async () => {
harness = await createRouteTestHarness(registerFileRoutes);
});
afterEach(() => {
vi.restoreAllMocks();
});
const get = async (query: string) => {
const res = await harness.app.inject({
method: 'GET',
url: `/api/sessions/${harness.ctx._sessionId}/files${query}`,
});
expect(res.statusCode).toBe(200);
return JSON.parse(res.body).data;
};
describe('over a workspace far bigger than the budget', () => {
const FOLDERS = 100;
const FILES_EACH = 50_000;
const WORKSPACE = '/virtual/ws';
let entriesServed = 0;
beforeEach(() => {
entriesServed = 0;
const folders = Array.from({ length: FOLDERS }, (_, index) =>
dirent(`class-${String(index).padStart(5, '0')}`, UV_DIRENT_DIR, WORKSPACE)
);
const files = Array.from({ length: FILES_EACH }, (_, index) =>
dirent(`image-${String(index).padStart(8, '0')}.jpg`, UV_DIRENT_FILE, 'x')
);
vi.spyOn(fs, 'readdir').mockImplementation((async (path: string) => {
const entries = path === WORKSPACE ? folders : files;
entriesServed += entries.length;
return [...entries];
}) as never);
vi.spyOn(fs, 'stat').mockImplementation((async (path: string) => ({
size: 1,
isDirectory: () => !String(path).endsWith('.jpg'),
})) as never);
harness.ctx._session.workingDir = WORKSPACE;
});
it('stops reading once it has read 250,000 entries, not at the end of the workspace', async () => {
const data = await get('');
// Past the ceiling by at most the folder that crossed it.
expect(entriesServed).toBeLessThanOrEqual(250_000 + FILES_EACH);
// What was read is still used, up to the budget, and nothing unread passes as empty.
expect(data.totalFiles + data.totalDirectories).toBe(5000);
expect(data.truncated).toBe(true);
expect(data.tree).toHaveLength(FOLDERS);
for (const folder of data.tree) expect(folder.incomplete).toBe(true);
expect(data.tree.filter((folder: { children: unknown[] }) => folder.children.length === 0)).toHaveLength(
FOLDERS - 5
);
});
it('stops a search the same way', async () => {
const data = await get('?q=image-00000000');
expect(entriesServed).toBeLessThanOrEqual(250_000 + FILES_EACH);
expect(data.matches.length).toBeGreaterThan(0);
expect(data.truncated).toBe(true);
});
});
describe('with a dead mount below the workspace', () => {
let workspace: string;
let deadFolder: string;
let deadReads: number;
let releaseStat: (() => void) | undefined;
beforeEach(() => {
workspace = mkdtempSync(join(tmpdir(), 'codeman-file-dead-'));
deadFolder = join(workspace, 'z-nas');
mkdirSync(deadFolder);
mkdirSync(join(workspace, 'a-fine'));
writeFileSync(join(workspace, 'a-fine', 'needle.ts'), '');
harness.ctx._session.workingDir = workspace;
deadReads = 0;
const never = new Promise<never>(() => {});
const realReaddir = fs.readdir.bind(fs);
const realStat = fs.stat.bind(fs);
vi.spyOn(fs, 'readdir').mockImplementation(((path: string, options: never) => {
if (path !== deadFolder) return realReaddir(path, options);
deadReads++;
return never;
}) as never);
vi.spyOn(fs, 'stat').mockImplementation(((path: string) => {
if (path !== deadFolder) return realStat(path);
// Settled in afterEach: the probe calls a path stalled for as long as its stat is pending.
return new Promise((_resolve, reject) => {
releaseStat = () => reject(Object.assign(new Error('gone'), { code: 'EIO' }));
});
}) as never);
vi.spyOn(console, 'warn').mockImplementation(() => {});
});
afterEach(async () => {
releaseStat?.();
releaseStat = undefined;
await new Promise((settled) => setTimeout(settled, 0));
rmSync(workspace, { recursive: true, force: true });
});
const deadline = <T>(work: Promise<T>): Promise<T> =>
Promise.race([
work,
new Promise<T>((_resolve, reject) => {
setTimeout(() => reject(new Error('the listing never answered')), 5000).unref();
}),
]);
it('answers without it, and costs it one stuck stat however often the panel asks', async () => {
const first = await deadline(get(''));
const dead = first.tree.find((node: { name: string }) => node.name === 'z-nas');
expect(dead).toEqual({ name: 'z-nas', path: 'z-nas', type: 'directory', children: [] });
expect(first.truncated).toBe(false);
expect(first.tree[0].children.map((node: { name: string }) => node.name)).toEqual(['needle.ts']);
expect(deadReads).toBe(0);
const statCalls = () => vi.mocked(fs.stat).mock.calls.filter(([path]) => path === deadFolder).length;
const stats = statCalls();
const search = await deadline(get('?q=needle'));
expect(search.matches.map((match: { path: string }) => match.path)).toEqual([join('a-fine', 'needle.ts')]);
expect(statCalls()).toBe(stats);
expect(deadReads).toBe(0);
});
});
});