From 64b20bdf7a89b47dd9b92c5a6cf3bede92af6828 Mon Sep 17 00:00:00 2001 From: Brennan Benson <79079362+brennanb2025@users.noreply.github.com> Date: Mon, 24 Aug 2026 10:57:48 -0700 Subject: [PATCH] fix skills visibility by provider registry --- .../use-native-chat-skills.test.ts | 31 ++++++++++--- .../native-chat/use-native-chat-skills.ts | 3 +- .../lib/orchestration-skill-coverage.test.ts | 6 +-- .../src/lib/orchestration-skill-coverage.ts | 12 +----- src/shared/skill-install-providers.ts | 43 +++++++++++++++++++ 5 files changed, 75 insertions(+), 20 deletions(-) diff --git a/src/renderer/src/components/native-chat/use-native-chat-skills.test.ts b/src/renderer/src/components/native-chat/use-native-chat-skills.test.ts index 6e5daaf43db..575b9808e1e 100644 --- a/src/renderer/src/components/native-chat/use-native-chat-skills.test.ts +++ b/src/renderer/src/components/native-chat/use-native-chat-skills.test.ts @@ -29,7 +29,7 @@ function discovery(owner: string | null, rootPath = '/Users/test/.agents/skills' id: 'source', label: 'Source', path: rootPath, - sourceKind: 'home' as const, + sourceKind: 'home' as 'home' | 'repo', providers: ['agent-skills' as const], owner, exists: true @@ -52,11 +52,30 @@ describe('isNativeChatSkillForAgent', () => { expect(isNativeChatSkillForAgent('claude', skill({ providers: ['agent-skills'] }))).toBe(false) }) - it('uses explicit source ownership and keeps shared roots visible', () => { + it('uses explicit source ownership and limits global shared roots to Codex', () => { const shared = discovery(null) expect(isNativeChatSkillForAgent('codex', skill({}), shared)).toBe(true) - expect(isNativeChatSkillForAgent('claude', skill({}), shared)).toBe(true) - expect(isNativeChatSkillForAgent('grok', skill({}), shared)).toBe(true) + expect(isNativeChatSkillForAgent('claude', skill({}), shared)).toBe(false) + expect(isNativeChatSkillForAgent('grok', skill({}), shared)).toBe(false) + }) + + it('allows canonical workspace roots only for providers that read them', () => { + const workspace = discovery(null, '/workspace/.agents/skills') + workspace.sources[0]!.sourceKind = 'repo' + expect( + isNativeChatSkillForAgent( + 'codex', + skill({ rootPath: '/workspace/.agents/skills', sourceKind: 'repo' }), + workspace + ) + ).toBe(true) + expect( + isNativeChatSkillForAgent( + 'claude', + skill({ rootPath: '/workspace/.agents/skills', sourceKind: 'repo' }), + workspace + ) + ).toBe(false) }) it('aliases OpenClaude to Claude roots without exposing them to other agents', () => { @@ -91,12 +110,12 @@ describe('isNativeChatSkillForAgent', () => { ] } satisfies Pick // A symlinked skill deduped under the Codex root but also reachable - // through the shared root stays visible to every agent. + // through the shared root stays visible to Codex only at global scope. const merged = skill({ rootPath: '/Users/test/.codex/skills', rootPaths: ['/Users/test/.codex/skills', '/Users/test/.agents/skills'] }) - expect(isNativeChatSkillForAgent('claude', merged, result)).toBe(true) + expect(isNativeChatSkillForAgent('claude', merged, result)).toBe(false) expect(isNativeChatSkillForAgent('codex', merged, result)).toBe(true) const codexOnly = skill({ rootPath: '/Users/test/.codex/skills', diff --git a/src/renderer/src/components/native-chat/use-native-chat-skills.ts b/src/renderer/src/components/native-chat/use-native-chat-skills.ts index f8a7e943935..7fa0087481f 100644 --- a/src/renderer/src/components/native-chat/use-native-chat-skills.ts +++ b/src/renderer/src/components/native-chat/use-native-chat-skills.ts @@ -11,6 +11,7 @@ import { selectNativeChatSkillStateInputs, type NativeChatSkillDiscoveryContext } from './native-chat-skill-discovery-context' +import { isSkillSourceVisibleToAgent } from '../../../../shared/skill-install-providers' export { resolveNativeChatSkillDiscoveryContext, @@ -66,7 +67,7 @@ export function isNativeChatSkillForAgent( const rootPaths = skill.rootPaths?.length ? skill.rootPaths : [skill.rootPath] return rootPaths.some((rootPath) => { const source = result.sources.find((entry) => entry.path === rootPath) - return source?.owner === null || source?.owner === profile.skillSourceOwner + return source ? isSkillSourceVisibleToAgent(profile.skillSourceOwner, source) : false }) } diff --git a/src/renderer/src/lib/orchestration-skill-coverage.test.ts b/src/renderer/src/lib/orchestration-skill-coverage.test.ts index d0128396e4e..668ddbeb30e 100644 --- a/src/renderer/src/lib/orchestration-skill-coverage.test.ts +++ b/src/renderer/src/lib/orchestration-skill-coverage.test.ts @@ -40,7 +40,7 @@ function source( } describe('orchestration skill agent coverage', () => { - it('marks shared-path agents from the global ~/.agents/skills install', () => { + it('marks only Codex from the global ~/.agents/skills install', () => { const skills = [ skill({ providers: ['agent-skills'], @@ -58,8 +58,8 @@ describe('orchestration skill agent coverage', () => { ) ).toEqual([ { agent: 'codex', label: 'Codex', installed: true }, - { agent: 'gemini', label: 'Gemini', installed: true }, - { agent: 'droid', label: 'Droid', installed: true } + { agent: 'gemini', label: 'Gemini', installed: false }, + { agent: 'droid', label: 'Droid', installed: false } ]) }) diff --git a/src/renderer/src/lib/orchestration-skill-coverage.ts b/src/renderer/src/lib/orchestration-skill-coverage.ts index 4196a50dd18..f44689dff01 100644 --- a/src/renderer/src/lib/orchestration-skill-coverage.ts +++ b/src/renderer/src/lib/orchestration-skill-coverage.ts @@ -1,9 +1,9 @@ -import type { AgentType } from '../../../shared/agent-status-types' import type { DiscoveredSkill, SkillDiscoverySource } from '../../../shared/skills' import type { TuiAgent } from '../../../shared/tui-agent' import { ORCHESTRATION_SKILL_NAME } from '@/lib/agent-feature-install-commands' import { getAgentLabel } from '@/lib/agent-catalog' import { TUI_AGENT_AUTO_PICK_ORDER } from '../../../shared/tui-agent-selection' +import { isSkillSourceVisibleToAgent } from '../../../shared/skill-install-providers' export type OrchestrationSkillAgentStatus = { agent: TuiAgent @@ -30,20 +30,12 @@ function isOrchestrationSkill(skill: DiscoveredSkill): boolean { ) } -// Why: `native-chat-agent-profiles.ts` carries the same rule as `skillSourceOwner`; -// keep the two in step when an agent starts reading another agent's roots. -function getSkillSourceOwnerForAgent(agent: TuiAgent): AgentType { - // Why: both launch Claude Code and therefore consume Claude-owned skill roots. - return agent === 'claude-agent-teams' || agent === 'openclaude' ? 'claude' : agent -} - /** `skills` and `sources` must come from the same discovery scan. */ export function agentHasOrchestrationSkill( agent: TuiAgent, skills: readonly DiscoveredSkill[], sources: readonly SkillDiscoverySource[] ): boolean { - const owner = getSkillSourceOwnerForAgent(agent) return skills.some((skill) => { if (!isOrchestrationSkill(skill)) { return false @@ -58,7 +50,7 @@ export function agentHasOrchestrationSkill( (source) => source.path === rootPath && source.sourceKind !== 'repo' && - (source.owner === null || source.owner === owner) + isSkillSourceVisibleToAgent(agent, source) ) ) }) diff --git a/src/shared/skill-install-providers.ts b/src/shared/skill-install-providers.ts index 6341b0a80cf..e63c12a9b33 100644 --- a/src/shared/skill-install-providers.ts +++ b/src/shared/skill-install-providers.ts @@ -1,3 +1,5 @@ +import type { SkillDiscoverySource } from './skills' + /** * Agents Orca can place an installed skill for. * @@ -95,6 +97,47 @@ export function skillInstallProvider( return PROVIDERS_BY_ID.get(id) } +const SKILL_INSTALL_PROVIDER_ALIASES: Readonly> = { + 'claude-agent-teams': 'claude', + openclaude: 'claude' +} + +/** Resolve a detected agent to the provider registry entry that owns its roots. */ +export function skillInstallProviderIdForAgent(value: string): SkillInstallProviderId | null { + if (isSkillInstallProviderId(value)) { + return value + } + return SKILL_INSTALL_PROVIDER_ALIASES[value] ?? null +} + +/** + * Whether a discovered source is readable by an agent at its scan scope. + * Shared canonical roots are only visible where the registry says the provider + * reads that scope; owned roots remain limited to their owner (and aliases). + */ +export function isSkillSourceVisibleToAgent( + agent: string, + source: Pick +): boolean { + const providerId = skillInstallProviderIdForAgent(agent) + const sourceOwner = source.owner + if (sourceOwner !== null) { + return sourceOwner === agent || sourceOwner === providerId + } + + const provider = providerId ? skillInstallProvider(providerId) : undefined + if (!provider) { + return false + } + if (source.sourceKind === 'home') { + return provider.globalSegments === null + } + if (source.sourceKind === 'repo') { + return provider.workspaceSegments === null + } + return false +} + /** Detected agents Orca can actually place skills for, in registry order. */ export function installableSkillProviders( detectedProviders: readonly string[]