diff --git a/src/main/skills/skill-plugin-cache-scan.test.ts b/src/main/skills/skill-plugin-cache-scan.test.ts index 442af48ce96..17f1c8e1eb9 100644 --- a/src/main/skills/skill-plugin-cache-scan.test.ts +++ b/src/main/skills/skill-plugin-cache-scan.test.ts @@ -4,7 +4,9 @@ import { tmpdir } from 'node:os' import { join } from 'node:path' import { promisify } from 'node:util' import { afterEach, describe, expect, it } from 'vitest' +import { isSkillScanIssueNeedingAttention } from '../../shared/skill-freshness' import { + MAXIMUM_PLUGIN_SCAN_ATTENTION_ISSUES, MAXIMUM_PLUGIN_SCAN_ISSUES, scanKnownPluginSkillCandidates } from './skill-plugin-cache-scan' @@ -463,6 +465,73 @@ describe('plugin skill candidate scan', () => { } ) + // Why: linking skills out of the cache is ordinary vendor packaging, so 'outside-root' is + // what fills the display budget on a real install — before any read failure is reached. + async function createCacheBehindSpentBudget(prefix: string, loopCount: number): Promise { + const parent = await mkdtemp(join(tmpdir(), prefix)) + temporaryDirectories.push(parent) + const root = join(parent, 'cache') + const outside = join(parent, 'outside') + await mkdir(outside, { recursive: true }) + await mkdir(root, { recursive: true }) + await Promise.all( + Array.from({ length: MAXIMUM_PLUGIN_SCAN_ISSUES }, (_, index) => + symlink(outside, join(root, `aa-linked-${index.toString().padStart(2, '0')}`), 'dir') + ) + ) + // Sorts after the links, so these are read once the budget is already spent. + await Promise.all( + Array.from({ length: loopCount }, (_, index) => { + const name = `zz-loop-${index.toString().padStart(2, '0')}` + return symlink(name, join(root, name), 'dir') + }) + ) + return root + } + + it.skipIf(process.platform === 'win32')( + 'keeps a read failure that lands after the display budget is spent', + async () => { + const root = await createCacheBehindSpentBudget('orca-plugin-attention-eviction-', 1) + + const result = await scanKnownPluginSkillCandidates(root, new Set(['orca-cli'])) + + // Why: a read failure is the only issue that can take the headline off "all up to + // date". Evicting it for display budget reports all-clear over a path that could be + // hiding a stale copy — the bounds that filled the budget say nothing about it. + expect(result.issues).toContainEqual({ + path: join(root, 'zz-loop-00'), + reason: 'io-error', + errorCode: 'ELOOP' + }) + const inventoryIssues = result.issues.map((issue) => ({ + rootId: 'plugin-cache', + sourceLabel: 'Plugin cache', + ...issue + })) + expect(inventoryIssues.some(isSkillScanIssueNeedingAttention)).toBe(true) + } + ) + + it.skipIf(process.platform === 'win32')( + 'bounds how many read failures outrank the display budget', + async () => { + const root = await createCacheBehindSpentBudget( + 'orca-plugin-attention-bound-', + MAXIMUM_PLUGIN_SCAN_ATTENTION_ISSUES + 4 + ) + + const result = await scanKnownPluginSkillCandidates(root, new Set(['orca-cli'])) + + // Why: outranking the budget is what makes this class unbounded, so a tree full of + // unreadable folders must not be able to pin one issue per folder in memory. + expect(result.issues.filter((issue) => issue.reason === 'io-error')).toHaveLength( + MAXIMUM_PLUGIN_SCAN_ATTENTION_ISSUES + ) + expect(result.issues).toContainEqual({ path: root, reason: 'issue-limit', errorCode: null }) + } + ) + it('reports the bound that ended the walk even with the issue budget spent', async () => { const root = await mkdtemp(join(tmpdir(), 'orca-plugin-truncating-issue-')) temporaryDirectories.push(root) diff --git a/src/main/skills/skill-plugin-cache-scan.ts b/src/main/skills/skill-plugin-cache-scan.ts index 78fbc53cd7b..7f151e8475b 100644 --- a/src/main/skills/skill-plugin-cache-scan.ts +++ b/src/main/skills/skill-plugin-cache-scan.ts @@ -2,6 +2,7 @@ import type { Dirent } from 'node:fs' import { opendir, realpath, stat } from 'node:fs/promises' import { basename, join } from 'node:path' import { + isSkillScanAttentionReason, isTruncatingSkillScanReason, type SkillFreshnessScanIssueReason } from '../../shared/skill-freshness' @@ -19,6 +20,11 @@ const MAXIMUM_NESTED_SKILL_DEPTH = 2 const MAXIMUM_PLUGIN_SCAN_ENTRIES = 16_384 export const MAXIMUM_PLUGIN_SKILL_CANDIDATES = 64 export const MAXIMUM_PLUGIN_SCAN_ISSUES = 16 +// Why: an attention issue outranks the display budget, so nothing else bounds how many a +// pathological tree can pin in memory. One is all the badge needs to be truthful; a few +// more give the dialog enough distinct paths to read as a pattern, and 'issue-limit' still +// says there are others. +export const MAXIMUM_PLUGIN_SCAN_ATTENTION_ISSUES = 4 const SKILL_FILE_NAME = 'SKILL.md' export type KnownPluginSkillCandidate = { @@ -52,6 +58,7 @@ export async function scanKnownPluginSkillCandidates( const issues: KnownPluginSkillScanIssue[] = [] const issueKeys = new Set() const visited = new Set() + let attentionIssueCount = 0 let resolvedRoot: string | null = null let entryCount = 0 let limitReached = false @@ -70,9 +77,15 @@ export async function scanKnownPluginSkillCandidates( if (issueKeys.has(key)) { return } + // Why: an attention issue is the only thing that can turn the headline off "all up to + // date", so evicting one for display budget makes Orca report all-clear over a read + // failure. Reserving a few keeps that unbounded on a tree full of unreadable folders. + const attention = + isSkillScanAttentionReason(reason) && + attentionIssueCount < MAXIMUM_PLUGIN_SCAN_ATTENTION_ISSUES // Why: the bound that ended the walk is the one issue the dialog cannot do without // — dropping it for display budget is what lets a truncated scan report all-clear. - const required = explainsCandidate || isTruncatingSkillScanReason(reason) + const required = explainsCandidate || attention || isTruncatingSkillScanReason(reason) // Why: this budget bounds what the dialog lists, not how far the scan reaches. // Ending the walk here would truncate coverage over a display limit — and since // Orca's own bounds no longer raise attention, it would do so silently. @@ -87,6 +100,9 @@ export async function scanKnownPluginSkillCandidates( return } issueKeys.add(key) + if (isSkillScanAttentionReason(reason)) { + attentionIssueCount += 1 + } issues.push({ path, reason, errorCode: code }) } diff --git a/src/shared/skill-freshness.ts b/src/shared/skill-freshness.ts index 0daddd4ed58..0414a1823c0 100644 --- a/src/shared/skill-freshness.ts +++ b/src/shared/skill-freshness.ts @@ -114,8 +114,12 @@ export type SkillFreshnessScanIssue = { // every installed skill. It is still listed in Details, like the other bounds. const SKILL_SCAN_ATTENTION_REASONS = new Set(['io-error']) +export function isSkillScanAttentionReason(reason: SkillFreshnessScanIssueReason): boolean { + return SKILL_SCAN_ATTENTION_REASONS.has(reason) +} + export function isSkillScanIssueNeedingAttention(issue: SkillFreshnessScanIssue): boolean { - return SKILL_SCAN_ATTENTION_REASONS.has(issue.reason) + return isSkillScanAttentionReason(issue.reason) } // Why: these are the bounds that end the walk rather than skip one folder. They are