perf(skills): batch repository dependency reads (#16377)

Batch dependency Markdown reads through the bounded Git snapshot reader and queue newly discovered dependencies once. Preserve package contents, limits, authorization, audits, and reference order.

Validation: 63 affected cases; full typecheck/build; exact-head full CI, E2E, Superagent and Apex 5/5. Local Git comparison: 502 individual reads reduced to three batches with identical entries.

Co-Authored-By: Paperclip <noreply@paperclip.ing>

server/src/__tests__/github-skill-source.test.ts

@@ -13,6 +13,28 @@ import { skillFileBytes } from '../services/skill-snapshot.js';
 const sha = 'a'.repeat(40);
 const md = (name: string) => `---\nname: ${name}\ndescription: A useful skill\n---\nFollow these instructions.\n`;
 describe('GitHub skill repository discovery', () => {
+  it('batches each dependency frontier instead of spawning a Git read for every Markdown file', async () => {
+    const files = {
+      ...Object.fromEntries(Array.from({ length: 64 }, (_, i) => [`skills/skill-${i}/SKILL.md`, md(`skill-${i}`) + '\n[Guide](../../shared/guide.md)'])),
+      'shared/guide.md': '[Details](details.md)',
+      'shared/details.md': '[Guide](guide.md) [Data](data.json)',
+      'shared/data.json': '{"example":true}',
+      ...Object.fromEntries(Array.from({ length: 10_000 }, (_, i) => [`unrelated/${i}.txt`, ''])),
+    };
+    const fixture = githubFixture(files);
+    const snapshot = await fixture.openSnapshot({ repositoryUrl: '', ref: 'main' });
+    const read = snapshot.readBlob;
+    const singleReads = vi.spyOn(snapshot, 'readBlob');
+    const batches = vi.fn(async (entries: typeof snapshot.entries) => new Map(await Promise.all(entries.map(async entry => [entry.sha, await read(entry)] as const))));
+    snapshot.readBlobs = batches;
+    fixture.openSnapshot = async () => snapshot;
+    const scan = await scanGitHubSkillsWithPackageFallback({ repositoryUrl: 'https://github.com/acme/skills', packageMode: 'repository' }, fixture);
+    expect(scan.repositoryFiles?.map(file => file.path).sort()).toEqual(Object.keys(files).filter(name => !name.startsWith('unrelated/')).sort());
+    expect(scan.candidates.every(skill => skill.error === null)).toBe(true);
+    expect(singleReads).not.toHaveBeenCalled();
+    expect(batches.mock.calls.map(([entries]) => entries.length)).toEqual([64, 1, 1, 1]);
+  });
+
   it('keeps skill folders and transitive shared files when unrelated repository files exceed the package limit', async () => {
     const fixture = githubFixture({
       'skills/main/SKILL.md': md('main') + '\nRead [shared](../../shared/guide.md) and [helper](../helper/SKILL.md).',

server/src/__tests__/skill-repository-dependencies.test.ts

@@ -0,0 +1,22 @@
+import { expect, it, vi } from 'vitest';
+import { skillRepositoryDependencies } from '../services/skill-repository-dependencies.js';
+import { MAX_REPOSITORY_FILES } from '../services/skill-repository-package.js';
+import type { GitSkillTreeEntry } from '../services/skill-source-git-snapshot.js';
+
+it('keeps path visits bounded for a reference chain at the repository file limit', async () => {
+  let pathVisits = 0;
+  const paths = ['skills/main/SKILL.md', ...Array.from({ length: MAX_REPOSITORY_FILES - 1 }, (_, i) => `shared/${i}.md`)];
+  const entries: GitSkillTreeEntry[] = paths.map((name, i) => ({
+    get path() { pathVisits++; return name; },
+    sha: i.toString(16).padStart(40, '0'), mode: '100644', type: 'blob', size: 100,
+  }));
+  const content = new Map(paths.map((name, i) => [name, Buffer.from(i === 0 ? '[Next](../../shared/0.md)'
+    : `[Next](${i === paths.length - 1 ? '0' : String(i)}.md)`)]));
+  const read = vi.fn(async (entry: GitSkillTreeEntry) => content.get(entry.path)!);
+  const prefetch = vi.fn(async () => {});
+  const selected = await skillRepositoryDependencies(entries, [entries[0]!], new Set(['skills/main']), read, prefetch);
+  expect(selected).toEqual(entries);
+  expect(read).toHaveBeenCalledTimes(MAX_REPOSITORY_FILES);
+  expect(prefetch).toHaveBeenCalledTimes(MAX_REPOSITORY_FILES);
+  expect(pathVisits).toBeLessThan(MAX_REPOSITORY_FILES * 100);
+});

server/src/__tests__/skill-source-git-snapshot.test.ts

@@ -25,7 +25,7 @@ vi.mock('node:child_process', async importOriginal => {
   } };
 });
 import { clearGitSkillSnapshotCache, openGitSkillSnapshot, parseGitDownloadProgress } from '../services/skill-source-git-snapshot.js';
-import { scanGitHubSkills, type GitHubRead } from '../services/github-skill-source.js';
+import { scanGitHubSkills, scanGitHubSkillsWithPackageFallback, type GitHubRead } from '../services/github-skill-source.js';
 const exec = promisify(execFile);
 let directory: string, commit: string, laterCommit: string;
 const git = (...args: string[]) => exec('git', ['-C', directory, '-c', 'user.name=Test', '-c', 'user.email=test@example.test', ...args]);
@@ -51,6 +51,45 @@ afterEach(async () => { vi.useRealTimers(); interception.hang = false; intercept
 afterAll(async () => { await rm(directory, { recursive: true, force: true }); });
 
 describe('Git skill snapshots', () => {
+  it('uses real Git batches for transitive dependencies in an oversized repository', async () => {
+    const remote = await mkdtemp(path.join(os.tmpdir(), 'paperclip-dependency-blobs-'));
+    interception.remote = remote;
+    try {
+      await exec('git', ['-C', remote, 'init', '-b', 'main']);
+      await mkdir(path.join(remote, 'unrelated'));
+      await mkdir(path.join(remote, 'shared'));
+      const files: Record<string, string> = {
+        'shared/guide.md': '[Details](details.md)',
+        'shared/details.md': '[Guide](guide.md) [Data](data.json)',
+        'shared/data.json': '{"value":true}',
+      };
+      for (let i = 0; i < 64; i++) {
+        await mkdir(path.join(remote, `skills/skill-${i}`), { recursive: true });
+        files[`skills/skill-${i}/SKILL.md`] = `---\nname: skill-${i}\ndescription: Dependency fixture\n---\n[Guide](../../shared/guide.md)`;
+      }
+      for (const [file, content] of Object.entries(files)) await writeFile(path.join(remote, file), content);
+      const unrelated = Buffer.alloc(1024 * 1024, 'x');
+      for (let i = 0; i < 62; i++) await writeFile(path.join(remote, `unrelated/${i}.bin`), unrelated);
+      await exec('git', ['-C', remote, 'add', '.']);
+      await exec('git', ['-C', remote, '-c', 'user.name=Test', '-c', 'user.email=test@example.test', 'commit', '-m', 'Dependency fixture']);
+      const commitSha = (await exec('git', ['-C', remote, 'rev-parse', 'HEAD'])).stdout.trim();
+      const read = (async (url: string) => url.includes('/commits/') ? { sha: commitSha } : { id: 42, full_name: 'acme/skills', default_branch: 'main' }) as GitHubRead;
+      read.openSnapshot = (input, options) => openGitSkillSnapshot({ ...request(), ref: input.ref, commitSha: input.commitSha }, options);
+      const scan = await scanGitHubSkillsWithPackageFallback({ repositoryUrl: 'https://github.com/acme/skills', packageMode: 'repository' }, read);
+      expect(scan.repositorySubset).toBe(true);
+      expect(scan.repositoryFiles?.map(file => file.path).sort()).toEqual(Object.keys(files).sort());
+      for (const file of scan.repositoryFiles!) {
+        const content = file.encoding === 'base64' ? Buffer.from(file.content, 'base64') : Buffer.from(file.content);
+        expect(content.toString()).toBe(files[file.path]);
+      }
+      const reads = interception.commands.filter(command => command.args.includes('cat-file'));
+      expect(reads).toHaveLength(4);
+      expect(reads.every(command => command.args.includes('--batch'))).toBe(true);
+    } finally {
+      await rm(remote, { recursive: true, force: true });
+    }
+  }, 30_000);
+
   it('reads many unrelated support files with one Git batch per scan, including cache hits', async () => {
     const remote = await mkdtemp(path.join(os.tmpdir(), 'paperclip-many-blobs-'));
     interception.remote = remote;

server/src/services/github-skill-source.ts

@@ -160,7 +160,11 @@ async function scanRepository(input: SkillScanInput, providerRead: GitHubRead, o
         if (dependencyBytes > MAX_REPOSITORY_BYTES) throw unprocessable('Repository package exceeds the 61 MB runtime limit.');
         dependencyBlobs.set(entry.sha, bytes);
         return bytes;
-      }) : entries.filter(entry => entry.type !== 'tree');
+      }, snapshot.readBlobs ? async pending => {
+        options.signal?.throwIfAborted();
+        const unread = pending.filter(entry => !dependencyBlobs.has(entry.sha) && (entry.size ?? 0) <= MAX_FILE_BYTES);
+        if (unread.length) for (const [sha, bytes] of await snapshot.readBlobs!(unread, options.signal)) dependencyBlobs.set(sha, bytes);
+      } : undefined) : entries.filter(entry => entry.type !== 'tree');
     if (repositoryInventory && repositoryInventory.length > MAX_REPOSITORY_FILES) throw unprocessable('Repository package exceeds the 9,990 file limit.');
     const readable = repositoryInventory?.filter(entry => entry.type === 'blob' && ['100644', '100755'].includes(entry.mode) && (entry.size ?? 0) <= MAX_FILE_BYTES);
     if (readable && readable.reduce((sum, entry) => sum + (entry.size ?? 0), 0) > MAX_REPOSITORY_BYTES) throw unprocessable('Repository package exceeds the 61 MB runtime limit.');

server/src/services/skill-repository-dependencies.ts

@@ -10,10 +10,12 @@ export async function skillRepositoryDependencies(
   initial: GitSkillTreeEntry[],
   skillDirectories: ReadonlySet<string>,
   readBlob: (entry: GitSkillTreeEntry) => Promise<Buffer | null>,
+  prefetch?: (entries: GitSkillTreeEntry[]) => Promise<void>,
 ): Promise<GitSkillTreeEntry[]> {
   const paths = indexSkillPackagePaths(entries.map(entry => entry.path));
   const byPath = new Map(entries.map(entry => [entry.path, entry]));
   const selected = new Map<string, GitSkillTreeEntry>();
+  let pending: GitSkillTreeEntry[] = [];
   let bytes = 0;
   const add = (entry: GitSkillTreeEntry) => {
     if (entry.type === 'tree' || selected.has(entry.path)) return;
@@ -21,26 +23,31 @@ export async function skillRepositoryDependencies(
     if (selected.size > MAX_REPOSITORY_FILES) throw unprocessable('Repository package exceeds the 9,990 file limit.');
     if (entry.type === 'blob' && ['100644', '100755'].includes(entry.mode) && (entry.size ?? 0) <= 1024 * 1024) bytes += entry.size ?? 0;
     if (bytes > MAX_REPOSITORY_BYTES) throw unprocessable('Repository package exceeds the 61 MB runtime limit.');
+    if (entry.type === 'blob' && ['100644', '100755'].includes(entry.mode) && /\.md$/i.test(entry.path)) pending.push(entry);
   };
   for (const entry of initial) add(entry);
   const manifest = byPath.get('paperclip.skills.json');
   if (manifest) add(manifest);
-  for (const entry of selected.values()) {
-    if (entry.type !== 'blob' || !['100644', '100755'].includes(entry.mode) || !/\.md$/i.test(entry.path)) continue;
-    const content = await readBlob(entry);
-    if (!content) continue;
-    let root = path.posix.dirname(entry.path);
-    while (root !== '.' && !skillDirectories.has(root)) root = path.posix.dirname(root);
-    const file = snapshotFile(path.posix.relative(root, entry.path), 'reference', content);
-    const inspection = inspectSkillPackage(path.posix.join(root, 'SKILL.md'), [file], paths, {}, [], new Set([entry.path]));
-    for (const reference of inspection.references) {
-      const target = reference.resolvedPath;
-      if (target === '..' || target.startsWith('../') || path.posix.isAbsolute(target)) continue;
-      const exact = byPath.get(target);
-      if (exact && exact.type !== 'tree') add(exact);
-      else if (paths.has(target)) {
-        const prefix = target === '.' ? '' : `${target}/`;
-        for (const child of entries) if (child.path.startsWith(prefix)) add(child);
+  while (pending.length) {
+    const frontier = pending;
+    pending = [];
+    await prefetch?.(frontier);
+    for (const entry of frontier) {
+      const content = await readBlob(entry);
+      if (!content) continue;
+      let root = path.posix.dirname(entry.path);
+      while (root !== '.' && !skillDirectories.has(root)) root = path.posix.dirname(root);
+      const file = snapshotFile(path.posix.relative(root, entry.path), 'reference', content);
+      const inspection = inspectSkillPackage(path.posix.join(root, 'SKILL.md'), [file], paths, {}, [], new Set([entry.path]));
+      for (const reference of inspection.references) {
+        const target = reference.resolvedPath;
+        if (target === '..' || target.startsWith('../') || path.posix.isAbsolute(target)) continue;
+        const exact = byPath.get(target);
+        if (exact && exact.type !== 'tree') add(exact);
+        else if (paths.has(target)) {
+          const prefix = target === '.' ? '' : `${target}/`;
+          for (const child of entries) if (child.path.startsWith(prefix)) add(child);
+        }
       }
     }
   }