diff --git a/src/main/skills/skill-plugin-cache-scan.test.ts b/src/main/skills/skill-plugin-cache-scan.test.ts index ed01d49cc..6434967aa 100644 --- a/src/main/skills/skill-plugin-cache-scan.test.ts +++ b/src/main/skills/skill-plugin-cache-scan.test.ts @@ -211,6 +211,48 @@ describe('plugin skill candidate scan', () => { expect(result).toEqual({ candidates: [], issues: [] }) }) + // Why this exists: the bound below is a deliberate tradeoff, and only the miss side of + // it was covered. Descending further would spend the entry budget on vendor payload — + // the cost that caused the false-attention root collapse in #10865 — while missing a + // copy only ever costs a Details row, because a plugin-cache placement is not + // convergeable by any update command. So the failure direction is silence, which is the + // safe one. Pinning BOTH sides means raising or lowering the bound has to be deliberate + // rather than an accident of refactoring. See #11454. + it.each([ + [0, true], + [1, true], + [2, true], + [3, false], + [4, false] + ])( + 'finds a nested skill %i level(s) below a package: %s', + async (intermediateDepth, expectFound) => { + const root = await mkdtemp(join(tmpdir(), 'orca-plugin-nested-depth-')) + temporaryDirectories.push(root) + const packageRoot = join(root, 'vendor', 'plugin', '1.0.0') + const hostSkill = join(packageRoot, 'skills', 'host-skill') + await mkdir(join(packageRoot, '.codex-plugin'), { recursive: true }) + await mkdir(hostSkill, { recursive: true }) + await writeFile(join(packageRoot, '.codex-plugin', 'plugin.json'), '{"skills":"./skills"}\n') + await writeFile(join(hostSkill, 'SKILL.md'), '# Host skill\n') + + let parent = hostSkill + for (let level = 1; level <= intermediateDepth; level += 1) { + parent = join(parent, `nested-${level}`) + } + const candidate = join(parent, 'orca-cli') + await mkdir(candidate, { recursive: true }) + await writeFile(join(candidate, 'SKILL.md'), '# Orca CLI\n') + + const result = await scanKnownPluginSkillCandidates(root, new Set(['orca-cli'])) + + expect(result.candidates).toEqual(expectFound ? [{ name: 'orca-cli', path: candidate }] : []) + // Why: pruned payload must never surface as a coverage issue either way — that is + // what keeps an ordinary large plugin cache from reporting a permanent problem. + expect(result.issues).toEqual([]) + } + ) + it('reports a depth-truncated subtree as scan coverage instead of a skill candidate', async () => { const root = await mkdtemp(join(tmpdir(), 'orca-plugin-skill-depth-')) temporaryDirectories.push(root) diff --git a/src/main/skills/skill-plugin-cache-scan.ts b/src/main/skills/skill-plugin-cache-scan.ts index 5cc93d473..029b5dd6f 100644 --- a/src/main/skills/skill-plugin-cache-scan.ts +++ b/src/main/skills/skill-plugin-cache-scan.ts @@ -13,6 +13,14 @@ const MAXIMUM_DECLARED_SKILL_SCAN_DEPTH = 6 // Why: a skill package's own payload (templates, fixtures, sample apps) is not a skill // tree, and it is what drives ordinary caches past the depth and entry bounds. Descend // far enough to still find a skill grouped under a package, then stop. +// +// Changing this is a real tradeoff, not a tuning knob. Raising it spends the entry budget +// on vendor payload — the cost that made ordinary caches collapse to a poison sentinel and +// pin every skill amber (#10865). Lowering it, or leaving it, means a skill buried deeper +// is never seen; that costs only a Details row, because a plugin-cache placement is not +// convergeable by any update command. So the failure direction here is silence, which is +// the safe one. Both sides of the boundary are pinned by test (#11454) — if you move this, +// that test will fail, and it is meant to. const MAXIMUM_NESTED_SKILL_DEPTH = 2 // Why: sized against a real multi-vendor cache, which reads ~7k entries once payload is // pruned. The bound still exists to stop a hostile or runaway tree; it is not a budget