diff --git a/config/scripts/generate-skill-bundle-manifest.mjs b/config/scripts/generate-skill-bundle-manifest.mjs index b091d17c5f8..47a3e5f84a8 100644 --- a/config/scripts/generate-skill-bundle-manifest.mjs +++ b/config/scripts/generate-skill-bundle-manifest.mjs @@ -152,35 +152,84 @@ async function collectPackageFiles(packageRoot) { return sortManifestFiles(files) } -function collectGitPackageFiles(ref, name) { - const sourcePrefix = `skills/${name}/` - const output = execFileSync('git', ['ls-tree', '-r', '-z', ref, '--', `skills/${name}`]) +function collectGitSkillTreeEntries(treeSha) { + const output = execFileSync('git', ['ls-tree', '-r', '-z', treeSha]) .toString('utf8') .split('\0') .filter(Boolean) - const caseFoldedPaths = new Map() - const files = output.map((line) => { + const packages = new Map() + for (const line of output) { const match = /^(\d+) (\w+) ([a-f0-9]+)\t(.+)$/.exec(line) if (!match) { - throw new Error(`Unexpected git tree entry at ${ref}: ${line}`) + throw new Error(`Unexpected git tree entry in ${treeSha}: ${line}`) } const [, mode, type, objectSha, sourcePath] = match + const separator = sourcePath.indexOf('/') + if (separator <= 0 || separator === sourcePath.length - 1) { + throw new Error(`Unsupported shipped skill path in ${treeSha}: ${sourcePath}`) + } + const name = sourcePath.slice(0, separator) + const manifestPath = sourcePath.slice(separator + 1) + const entries = packages.get(name) ?? [] + entries.push({ mode, type, objectSha, manifestPath }) + packages.set(name, entries) + } + return packages +} + +function readGitBlobs(objectShas) { + const uniqueShas = [...new Set(objectShas)] + if (uniqueShas.length === 0) { + return new Map() + } + // Why: released history spans hundreds of tags. Batch mode avoids a Git + // subprocess per historical file while remaining available on Git 2.25. + const output = execFileSync('git', ['cat-file', '--batch'], { + input: `${uniqueShas.join('\n')}\n`, + maxBuffer: 64 * 1024 * 1024 + }) + const blobs = new Map() + let offset = 0 + for (const requestedSha of uniqueShas) { + const headerEnd = output.indexOf(10, offset) + if (headerEnd < 0) { + throw new Error(`Missing git cat-file header for ${requestedSha}`) + } + const header = output.subarray(offset, headerEnd).toString('utf8') + const match = /^([a-f0-9]+) blob (\d+)$/.exec(header) + if (!match || match[1] !== requestedSha) { + throw new Error(`Unexpected git cat-file header for ${requestedSha}: ${header}`) + } + const size = Number(match[2]) + const contentStart = headerEnd + 1 + const contentEnd = contentStart + size + if (contentEnd >= output.length || output[contentEnd] !== 10) { + throw new Error(`Truncated git blob for ${requestedSha}`) + } + blobs.set(requestedSha, Buffer.from(output.subarray(contentStart, contentEnd))) + offset = contentEnd + 1 + } + return blobs +} + +function collectGitPackageFiles(treeSha, name, entries, blobs) { + const caseFoldedPaths = new Map() + const files = entries.map(({ mode, type, objectSha, manifestPath }) => { if (type !== 'blob' || (mode !== '100644' && mode !== '100755')) { - throw new Error(`Unsupported shipped skill entry at ${ref}: ${line}`) + throw new Error(`Unsupported shipped skill entry in ${treeSha}: ${name}/${manifestPath}`) } - const manifestPath = sourcePath.slice(sourcePrefix.length) assertSafeRelativePath(manifestPath) const foldedPath = manifestPath.toLocaleLowerCase('en-US') const collision = caseFoldedPaths.get(foldedPath) if (collision && collision !== manifestPath) { - throw new Error(`Case-colliding skill paths at ${ref}: ${collision} and ${manifestPath}`) + throw new Error(`Case-colliding skill paths in ${treeSha}: ${collision} and ${manifestPath}`) } caseFoldedPaths.set(foldedPath, manifestPath) - return describeFile( - manifestPath, - execFileSync('git', ['cat-file', 'blob', objectSha]), - mode === '100755' - ) + const bytes = blobs.get(objectSha) + if (!bytes) { + throw new Error(`Missing git blob ${objectSha} for ${name}/${manifestPath}`) + } + return describeFile(manifestPath, bytes, mode === '100755') }) // Why: git ls-tree emits git byte-order, not the canonical walk order. return sortManifestFiles(files) @@ -232,45 +281,61 @@ function releaseTags() { .filter((tag) => /^v\d+\.\d+\.\d+(?:[-.][0-9A-Za-z.-]+)?$/.test(tag)) } -function skillsTreeShaAtRef(ref) { - try { - const output = execFileSync('git', ['ls-tree', ref, 'skills'], { - encoding: 'utf8', - stdio: ['ignore', 'pipe', 'ignore'] - }).trim() - return output ? output.split(/\s+/)[2] : null - } catch { - return null - } -} - -function skillNamesAtRef(ref) { - try { - return execFileSync('git', ['ls-tree', '-d', '--name-only', `${ref}:skills`], { - encoding: 'utf8', - stdio: ['ignore', 'pipe', 'ignore'] - }) - .split('\n') - .filter(Boolean) - .sort((left, right) => left.localeCompare(right, 'en')) - } catch { +function skillsTreeShasAtRefs(refs) { + if (refs.length === 0) { return [] } + const output = execFileSync('git', ['cat-file', '--batch-check=%(objectname) %(objecttype)'], { + input: `${refs.map((ref) => `${ref}:skills`).join('\n')}\n`, + encoding: 'utf8' + }) + const lines = output.trimEnd().split('\n') + if (lines.length !== refs.length) { + throw new Error(`Expected ${refs.length} skills tree identities, received ${lines.length}`) + } + return lines.map((line, index) => { + if (line.endsWith(' missing')) { + return null + } + const match = /^([a-f0-9]+) tree$/.exec(line) + if (!match) { + throw new Error(`Unexpected skills tree identity at ${refs[index]}: ${line}`) + } + return match[1] + }) } function buildReleasedHistory() { const registry = { schemaVersion: SCHEMA_VERSION, skills: {} } const mapping = { schemaVersion: SCHEMA_VERSION, releases: [] } + const tags = releaseTags() + const treeShas = skillsTreeShasAtRefs(tags) + const distinctTreeShas = [...new Set(treeShas.filter(Boolean))] + const packagesByTree = new Map( + distinctTreeShas.map((treeSha) => [treeSha, collectGitSkillTreeEntries(treeSha)]) + ) + const blobs = readGitBlobs( + [...packagesByTree.values()].flatMap((packages) => + [...packages.values()].flatMap((entries) => entries.map((entry) => entry.objectSha)) + ) + ) let previousSkillsTreeSha = null - for (const tag of releaseTags()) { - const skillsTreeSha = skillsTreeShaAtRef(tag) + for (const [index, tag] of tags.entries()) { + const skillsTreeSha = treeShas[index] if (!skillsTreeSha || skillsTreeSha === previousSkillsTreeSha) { continue } previousSkillsTreeSha = skillsTreeSha const revisions = {} - for (const name of skillNamesAtRef(tag)) { - const filesWithGitHashes = collectGitPackageFiles(tag, name) + const packages = packagesByTree.get(skillsTreeSha) + if (!packages) { + throw new Error(`Missing released skill tree ${skillsTreeSha} at ${tag}`) + } + for (const name of [...packages.keys()].sort((left, right) => + left.localeCompare(right, 'en') + )) { + const entries = packages.get(name) + const filesWithGitHashes = collectGitPackageFiles(skillsTreeSha, name, entries, blobs) if (!filesWithGitHashes.some((file) => file.path === 'SKILL.md')) { continue } diff --git a/src/main/skills/skill-freshness-inventory-limits.test.ts b/src/main/skills/skill-freshness-inventory-limits.test.ts new file mode 100644 index 00000000000..c43d137fcd0 --- /dev/null +++ b/src/main/skills/skill-freshness-inventory-limits.test.ts @@ -0,0 +1,26 @@ +import { describe, expect, it } from 'vitest' +import { + boundRepositorySkillRoots, + MAXIMUM_REPOSITORY_SKILL_ROOTS +} from './skill-freshness-inventory' +import type { SkillScanRoot } from './skill-discovery-sources' + +describe('skill freshness inventory limits', () => { + it('caps repository roots before creating candidate probes', () => { + const roots = Array.from( + { length: MAXIMUM_REPOSITORY_SKILL_ROOTS + 3 }, + (_, index): SkillScanRoot => ({ + id: `repo-${index}`, + label: `Repo ${index}`, + path: `/repo-${index}/.agents/skills`, + sourceKind: 'repo', + providers: ['agent-skills'] + }) + ) + + const bounded = boundRepositorySkillRoots(roots) + + expect(bounded.scanned).toHaveLength(MAXIMUM_REPOSITORY_SKILL_ROOTS) + expect(bounded.omitted).toHaveLength(3) + }) +}) diff --git a/src/main/skills/skill-freshness-inventory.test.ts b/src/main/skills/skill-freshness-inventory.test.ts index 40054faa4df..d80a0307667 100644 --- a/src/main/skills/skill-freshness-inventory.test.ts +++ b/src/main/skills/skill-freshness-inventory.test.ts @@ -8,7 +8,10 @@ import type { SkillCurrentBundleEntry, SkillKnownSnapshot } from '../../shared/skill-freshness' -import { inventorySkillFreshness } from './skill-freshness-inventory' +import { + inventorySkillFreshness, + MAXIMUM_REPOSITORY_SKILL_ROOTS +} from './skill-freshness-inventory' import { describeObservedSkillFile, skillPackageDigest } from './skill-package-identity' const temporaryDirectories: string[] = [] @@ -314,4 +317,26 @@ describe('read-only skill freshness inventory', () => { installedAppVersion: '2.0.0' }) }) + + it('withholds updates when stored repositories exceed the probe budget', async () => { + const test = await fixture() + await test.writeSkill(join(test.homeDir, '.agents', 'skills'), test.oldMarkdown) + const repos = Array.from( + { length: MAXIMUM_REPOSITORY_SKILL_ROOTS / 2 + 1 }, + (_, index) => ({ id: `repo-${index}`, path: join(test.root, `repo-${index}`) }) as Repo + ) + + const inventory = await inventorySkillFreshness({ + homeDir: test.homeDir, + repos, + resourceRoot: test.resourceRoot + }) + + expect(inventory.installations).toEqual( + expect.arrayContaining([ + expect.objectContaining({ errorCategory: 'repository-scan-limit', status: 'inaccessible' }) + ]) + ) + expect(inventory.eligibleUpdateNames).toEqual([]) + }) }) diff --git a/src/main/skills/skill-freshness-inventory.ts b/src/main/skills/skill-freshness-inventory.ts index 6ec3441fd4f..e5bbae31da9 100644 --- a/src/main/skills/skill-freshness-inventory.ts +++ b/src/main/skills/skill-freshness-inventory.ts @@ -5,7 +5,7 @@ import type { SkillFreshnessInstallation, SkillFreshnessInventory } from '../../shared/skill-freshness' -import { buildSkillDiscoverySources } from './skill-discovery-sources' +import { buildSkillDiscoverySources, type SkillScanRoot } from './skill-discovery-sources' import { loadSkillBundleArtifacts } from './skill-bundle-artifacts' import { eligibleSkillUpdateNames } from './skill-freshness-eligibility' import { runSkillCandidateTasks } from './skill-candidate-concurrency' @@ -18,6 +18,18 @@ import { } from './skill-freshness-placement-observation' import { scanKnownPluginSkillCandidates } from './skill-plugin-cache-scan' +export const MAXIMUM_REPOSITORY_SKILL_ROOTS = 128 + +export function boundRepositorySkillRoots(roots: readonly SkillScanRoot[]): { + scanned: SkillScanRoot[] + omitted: SkillScanRoot[] +} { + return { + scanned: roots.slice(0, MAXIMUM_REPOSITORY_SKILL_ROOTS), + omitted: roots.slice(MAXIMUM_REPOSITORY_SKILL_ROOTS) + } +} + export async function inventorySkillFreshness( args: { homeDir?: string @@ -39,7 +51,8 @@ export async function inventorySkillFreshness( } const roots = buildSkillDiscoverySources(discoveryArgs) const homeRoots = roots.filter((root) => root.sourceKind === 'home') - const repoRoots = roots.filter((root) => root.sourceKind === 'repo') + const allRepoRoots = roots.filter((root) => root.sourceKind === 'repo') + const { scanned: repoRoots, omitted: omittedRepoRoots } = boundRepositorySkillRoots(allRepoRoots) const pluginRoots = roots.filter((root) => root.sourceKind === 'plugin') const canonicalRootPath = homeRoots.find((root) => root.id === 'home-agents')?.path if (!canonicalRootPath) { @@ -77,6 +90,29 @@ export async function inventorySkillFreshness( }) ) ) + // Why: stored repositories can grow without bound. If the probe budget is + // exhausted, one sentinel per name preserves safety without hashing more packages. + const omittedRepoTasks = + omittedRepoRoots.length === 0 + ? [] + : artifacts.manifest.skills.map( + (current) => () => + observeSkillFreshnessInstallation({ + current, + artifacts, + rootId: 'repo-scan-limit', + providers: [...new Set(omittedRepoRoots.flatMap((root) => root.providers))], + sourceKind: 'repo', + sourceLabel: 'Additional repositories', + unresolvedPath: omittedRepoRoots[0]?.path ?? 'repo-scan-limit', + topology: { + topology: 'repo-scope', + resolvedPath: null, + identity: null, + errorCategory: 'repository-scan-limit' + } + }) + ) const pluginScans = await Promise.all( pluginRoots.map(async (root) => ({ root, @@ -123,7 +159,7 @@ export async function inventorySkillFreshness( ) ]) const unsupportedInstallations = ( - await runSkillCandidateTasks([...repoTasks, ...pluginTasks]) + await runSkillCandidateTasks([...repoTasks, ...omittedRepoTasks, ...pluginTasks]) ).filter((installation): installation is SkillFreshnessInstallation => installation !== null) const installations = dedupeSkillFreshnessPlacements([ ...homeInstallations, diff --git a/src/main/skills/skill-plugin-cache-scan.test.ts b/src/main/skills/skill-plugin-cache-scan.test.ts new file mode 100644 index 00000000000..90c71b2d470 --- /dev/null +++ b/src/main/skills/skill-plugin-cache-scan.test.ts @@ -0,0 +1,26 @@ +import { mkdir, mkdtemp, rm } from 'node:fs/promises' +import { tmpdir } from 'node:os' +import { join } from 'node:path' +import { afterEach, describe, expect, it } from 'vitest' +import { scanKnownPluginSkillCandidates } from './skill-plugin-cache-scan' + +const temporaryDirectories: string[] = [] + +afterEach(async () => { + await Promise.all(temporaryDirectories.splice(0).map((root) => rm(root, { recursive: true }))) +}) + +describe('plugin skill candidate scan', () => { + it('stops at the package candidate budget and marks the scan incomplete', async () => { + const root = await mkdtemp(join(tmpdir(), 'orca-plugin-skill-scan-')) + temporaryDirectories.push(root) + await Promise.all( + ['one', 'two'].map((vendor) => mkdir(join(root, vendor, 'orca-cli'), { recursive: true })) + ) + + const result = await scanKnownPluginSkillCandidates(root, new Set(['orca-cli']), 1) + + expect(result.candidates).toHaveLength(1) + expect(result.incompletePaths).toEqual([root]) + }) +}) diff --git a/src/main/skills/skill-plugin-cache-scan.ts b/src/main/skills/skill-plugin-cache-scan.ts index 2fcc9dca67d..3a8ec05924d 100644 --- a/src/main/skills/skill-plugin-cache-scan.ts +++ b/src/main/skills/skill-plugin-cache-scan.ts @@ -4,6 +4,8 @@ import { join } from 'node:path' const MAXIMUM_PLUGIN_SCAN_DEPTH = 9 const MAXIMUM_PLUGIN_SCAN_ENTRIES = 4_096 +export const MAXIMUM_PLUGIN_SKILL_CANDIDATES = 64 +const MAXIMUM_PLUGIN_INCOMPLETE_PATHS = 16 export type KnownPluginSkillCandidate = { name: string @@ -23,7 +25,8 @@ function errorCode(error: unknown): string | null { export async function scanKnownPluginSkillCandidates( rootPath: string, - knownNames: ReadonlySet + knownNames: ReadonlySet, + maximumCandidates = MAXIMUM_PLUGIN_SKILL_CANDIDATES ): Promise { const candidates: KnownPluginSkillCandidate[] = [] const incompletePaths = new Set() @@ -31,6 +34,21 @@ export async function scanKnownPluginSkillCandidates( let entryCount = 0 let limitReached = false + function recordIncomplete(path: string): void { + if (incompletePaths.has(path)) { + return + } + if (incompletePaths.size >= MAXIMUM_PLUGIN_INCOMPLETE_PATHS) { + // Why: each incomplete path expands to one conservative row per official + // skill. Collapse a hostile cache into one poison sentinel before IPC/render fanout. + incompletePaths.clear() + incompletePaths.add(rootPath) + limitReached = true + return + } + incompletePaths.add(path) + } + async function visit(directory: string, depth: number): Promise { if (limitReached || depth > MAXIMUM_PLUGIN_SCAN_DEPTH) { return @@ -40,7 +58,7 @@ export async function scanKnownPluginSkillCandidates( resolved = await realpath(directory) } catch (error) { if (errorCode(error) !== 'ENOENT') { - incompletePaths.add(directory) + recordIncomplete(directory) } return } @@ -53,7 +71,7 @@ export async function scanKnownPluginSkillCandidates( try { handle = await opendir(directory) } catch { - incompletePaths.add(directory) + recordIncomplete(directory) return } const entries: Dirent[] = [] @@ -66,17 +84,18 @@ export async function scanKnownPluginSkillCandidates( entryCount += 1 if (entryCount > MAXIMUM_PLUGIN_SCAN_ENTRIES) { limitReached = true - incompletePaths.add(rootPath) + recordIncomplete(rootPath) break } entries.push(entry) } } catch { - incompletePaths.add(directory) + recordIncomplete(directory) } finally { await handle.close().catch(() => undefined) } + entries.sort((left, right) => left.name.localeCompare(right.name, 'en')) for (const entry of entries) { if (limitReached) { return @@ -88,6 +107,11 @@ export async function scanKnownPluginSkillCandidates( directoryEntry = (await stat(entryPath)).isDirectory() } catch { if (knownNames.has(entry.name)) { + if (candidates.length >= maximumCandidates) { + limitReached = true + recordIncomplete(rootPath) + return + } candidates.push({ name: entry.name, path: entryPath }) } continue @@ -97,6 +121,11 @@ export async function scanKnownPluginSkillCandidates( continue } if (knownNames.has(entry.name)) { + if (candidates.length >= maximumCandidates) { + limitReached = true + recordIncomplete(rootPath) + return + } candidates.push({ name: entry.name, path: entryPath }) continue }