fix(skills): stop the scan issue budget evicting a read failure (#11221)

The per-scan issue budget kept an issue only when it explained a candidate
or truncated the walk. Neither set intersects the attention set, so
'io-error' — the sole reason a plugin-cache scan can raise "Needs
attention" — was droppable. Once 16 ordinary issues filled the budget (16
'outside-root' vendor symlinks is an install shape the scan itself
documents as normal), a later read failure was evicted for a generic
'issue-limit' row that raises neither attention nor truncation, and the
dialog headline read "All installed Orca skills are up to date" over a
path that could be hiding a stale copy.

Attention issues now outrank the budget, capped at a small reserve so an
adversarial tree of unreadable folders cannot pin one issue per folder.

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<string> {
+    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)

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<string>()
   const visited = new Set<string>()
+  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 })
   }
 

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<SkillFreshnessScanIssueReason>(['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