mirror of
https://github.com/iflytek/skillhub.git
synced 2026-09-10 22:41:02 +00:00
feat(cli,domain): support non-global namespace skill download (#497)
* feat(cli,domain): support non-global namespace skill download Parse namespace from skill name using -- separator (e.g., astroclaw--api-gateway) so users don't need --namespace flag. Allow anonymous download for any PUBLIC skill regardless of namespace. CLI changes: - Add cli/src/shared/skill-name-parser.ts utility - Update install and remove commands to parse skill name argument - 10 unit tests covering edge cases Domain changes: - SkillDownloadService.isAnonymousDownloadAllowed: drop namespace type check, only require PUBLIC visibility - Update test to expect success for team-namespace public skill Synced from SAAS commit 26c67e31b1221249cf9b73321d1b726d8ba6e6df * fix(cli): use bun:test instead of vitest in skill-name-parser test
This commit is contained in:
parent
04caf21e76
commit
2bb7dedaf4
6 changed files with 151 additions and 16 deletions
|
|
@ -5,6 +5,7 @@ import { installSkill } from '../services/install-service'
|
|||
import { resolveInstallTargets } from '../agents/resolver'
|
||||
import { CliError } from '../shared/errors'
|
||||
import { EXIT } from '../shared/constants'
|
||||
import { parseSkillName } from '../shared/skill-name-parser'
|
||||
|
||||
export interface InstallCommandOptions {
|
||||
namespace?: string | undefined
|
||||
|
|
@ -74,7 +75,7 @@ async function defaultPromptScope(): Promise<'user' | 'project'> {
|
|||
}
|
||||
|
||||
export async function installCommand(
|
||||
slug: string,
|
||||
skillNameArg: string,
|
||||
options: InstallCommandOptions,
|
||||
deps: InstallCommandDeps = {}
|
||||
): Promise<string> {
|
||||
|
|
@ -92,7 +93,10 @@ export async function installCommand(
|
|||
const credentialsStore = new CredentialsStore()
|
||||
const registry = resolveRegistry(options, process.env, await configStore.read())
|
||||
const token = resolveToken(options, process.env, await credentialsStore.getToken(registry))
|
||||
const namespace = options.namespace ?? 'global'
|
||||
|
||||
const parsed = parseSkillName(skillNameArg)
|
||||
const namespace = options.namespace ?? parsed.namespace
|
||||
const slug = parsed.slug
|
||||
|
||||
const resolveTargets = deps.resolveInstallTargets ?? resolveInstallTargets
|
||||
const targets = await resolveTargets({
|
||||
|
|
|
|||
|
|
@ -5,6 +5,7 @@ import { resolveRegistry, resolveToken } from '../services/registry-service'
|
|||
import { removeLocalSkill } from '../services/remove-service'
|
||||
import { CliError } from '../shared/errors'
|
||||
import { EXIT } from '../shared/constants'
|
||||
import { parseSkillName } from '../shared/skill-name-parser'
|
||||
|
||||
export interface RemoveCommandOptions {
|
||||
agent?: string[] | undefined
|
||||
|
|
@ -17,7 +18,7 @@ export interface RemoveCommandOptions {
|
|||
json?: boolean | undefined
|
||||
}
|
||||
|
||||
export async function removeCommand(slug: string, options: RemoveCommandOptions): Promise<string> {
|
||||
export async function removeCommand(skillNameArg: string, options: RemoveCommandOptions): Promise<string> {
|
||||
if (options.all && options.agent?.length) {
|
||||
throw new CliError('--all cannot be used with --agent', EXIT.usage)
|
||||
}
|
||||
|
|
@ -29,9 +30,12 @@ export async function removeCommand(slug: string, options: RemoveCommandOptions)
|
|||
const credentialsStore = new CredentialsStore()
|
||||
const registry = resolveRegistry(options, process.env, await configStore.read())
|
||||
|
||||
const parsed = parseSkillName(skillNameArg)
|
||||
const namespace = options.namespace ?? parsed.namespace
|
||||
const slug = parsed.slug
|
||||
|
||||
if (options.remote) {
|
||||
const token = resolveToken(options, process.env, await credentialsStore.getToken(registry))
|
||||
const namespace = options.namespace ?? 'global'
|
||||
|
||||
if (!options.hard && process.stdout.isTTY) {
|
||||
const prompts = await import('prompts')
|
||||
|
|
|
|||
27
cli/src/shared/skill-name-parser.ts
Normal file
27
cli/src/shared/skill-name-parser.ts
Normal file
|
|
@ -0,0 +1,27 @@
|
|||
export interface ParsedSkillName {
|
||||
namespace: string
|
||||
slug: string
|
||||
}
|
||||
|
||||
export function parseSkillName(skillName: string, defaultNamespace = 'global'): ParsedSkillName {
|
||||
const separatorIndex = skillName.indexOf('--')
|
||||
|
||||
if (separatorIndex <= 0) {
|
||||
return {
|
||||
namespace: defaultNamespace,
|
||||
slug: separatorIndex === 0 ? skillName.slice(2) : skillName
|
||||
}
|
||||
}
|
||||
|
||||
if (separatorIndex === skillName.length - 2) {
|
||||
return {
|
||||
namespace: defaultNamespace,
|
||||
slug: skillName.slice(0, -2)
|
||||
}
|
||||
}
|
||||
|
||||
return {
|
||||
namespace: skillName.slice(0, separatorIndex),
|
||||
slug: skillName.slice(separatorIndex + 2)
|
||||
}
|
||||
}
|
||||
90
cli/test/unit/shared/skill-name-parser.test.ts
Normal file
90
cli/test/unit/shared/skill-name-parser.test.ts
Normal file
|
|
@ -0,0 +1,90 @@
|
|||
import { describe, test, expect } from 'bun:test'
|
||||
import { parseSkillName } from '../../../src/shared/skill-name-parser'
|
||||
|
||||
describe('parseSkillName', () => {
|
||||
describe('with namespace--slug format', () => {
|
||||
test('should parse namespace and slug separated by double dash', () => {
|
||||
const result = parseSkillName('astroclaw--api-gateway')
|
||||
expect(result).toEqual({
|
||||
namespace: 'astroclaw',
|
||||
slug: 'api-gateway'
|
||||
})
|
||||
})
|
||||
|
||||
test('should handle namespace and slug with single dashes', () => {
|
||||
const result = parseSkillName('my-org--my-skill-name')
|
||||
expect(result).toEqual({
|
||||
namespace: 'my-org',
|
||||
slug: 'my-skill-name'
|
||||
})
|
||||
})
|
||||
|
||||
test('should handle multiple double dashes by using first as separator', () => {
|
||||
const result = parseSkillName('namespace--slug--with--dashes')
|
||||
expect(result).toEqual({
|
||||
namespace: 'namespace',
|
||||
slug: 'slug--with--dashes'
|
||||
})
|
||||
})
|
||||
})
|
||||
|
||||
describe('with slug only format', () => {
|
||||
test('should use default namespace when no separator present', () => {
|
||||
const result = parseSkillName('api-gateway')
|
||||
expect(result).toEqual({
|
||||
namespace: 'global',
|
||||
slug: 'api-gateway'
|
||||
})
|
||||
})
|
||||
|
||||
test('should use custom default namespace when provided', () => {
|
||||
const result = parseSkillName('api-gateway', 'myorg')
|
||||
expect(result).toEqual({
|
||||
namespace: 'myorg',
|
||||
slug: 'api-gateway'
|
||||
})
|
||||
})
|
||||
|
||||
test('should handle slug with single dashes', () => {
|
||||
const result = parseSkillName('my-skill-name')
|
||||
expect(result).toEqual({
|
||||
namespace: 'global',
|
||||
slug: 'my-skill-name'
|
||||
})
|
||||
})
|
||||
})
|
||||
|
||||
describe('edge cases', () => {
|
||||
test('should handle separator at start', () => {
|
||||
const result = parseSkillName('--api-gateway')
|
||||
expect(result).toEqual({
|
||||
namespace: 'global',
|
||||
slug: 'api-gateway'
|
||||
})
|
||||
})
|
||||
|
||||
test('should handle separator at end', () => {
|
||||
const result = parseSkillName('astroclaw--')
|
||||
expect(result).toEqual({
|
||||
namespace: 'global',
|
||||
slug: 'astroclaw'
|
||||
})
|
||||
})
|
||||
|
||||
test('should handle empty string', () => {
|
||||
const result = parseSkillName('')
|
||||
expect(result).toEqual({
|
||||
namespace: 'global',
|
||||
slug: ''
|
||||
})
|
||||
})
|
||||
|
||||
test('should handle just separator', () => {
|
||||
const result = parseSkillName('--')
|
||||
expect(result).toEqual({
|
||||
namespace: 'global',
|
||||
slug: ''
|
||||
})
|
||||
})
|
||||
})
|
||||
})
|
||||
|
|
@ -4,7 +4,6 @@ import com.iflytek.skillhub.domain.event.SkillDownloadedEvent;
|
|||
import com.iflytek.skillhub.domain.namespace.Namespace;
|
||||
import com.iflytek.skillhub.domain.namespace.NamespaceRepository;
|
||||
import com.iflytek.skillhub.domain.namespace.NamespaceRole;
|
||||
import com.iflytek.skillhub.domain.namespace.NamespaceType;
|
||||
import com.iflytek.skillhub.domain.shared.exception.DomainBadRequestException;
|
||||
import com.iflytek.skillhub.domain.shared.exception.DomainForbiddenException;
|
||||
import com.iflytek.skillhub.domain.skill.*;
|
||||
|
|
@ -268,7 +267,7 @@ public class SkillDownloadService {
|
|||
Skill skill,
|
||||
String currentUserId,
|
||||
Map<Long, NamespaceRole> userNsRoles) {
|
||||
if (currentUserId == null && !isAnonymousDownloadAllowed(namespace, skill)) {
|
||||
if (currentUserId == null && !isAnonymousDownloadAllowed(skill)) {
|
||||
throw new DomainForbiddenException("error.skill.access.denied", skill.getSlug());
|
||||
}
|
||||
if (!visibilityChecker.canAccess(skill, currentUserId, userNsRoles)) {
|
||||
|
|
@ -276,9 +275,8 @@ public class SkillDownloadService {
|
|||
}
|
||||
}
|
||||
|
||||
private boolean isAnonymousDownloadAllowed(Namespace namespace, Skill skill) {
|
||||
return namespace.getType() == NamespaceType.GLOBAL
|
||||
&& skill.getVisibility() == SkillVisibility.PUBLIC;
|
||||
private boolean isAnonymousDownloadAllowed(Skill skill) {
|
||||
return skill.getVisibility() == SkillVisibility.PUBLIC;
|
||||
}
|
||||
|
||||
private Skill resolveVisibleSkill(Long namespaceId, String slug, String currentUserId) {
|
||||
|
|
|
|||
|
|
@ -318,26 +318,38 @@ class SkillDownloadServiceTest {
|
|||
}
|
||||
|
||||
@Test
|
||||
void testDownloadVersion_RejectsAnonymousForTeamNamespacePublicSkill() throws Exception {
|
||||
void testDownloadVersion_AllowsAnonymousForTeamNamespacePublicSkill() throws Exception {
|
||||
Namespace namespace = new Namespace("team-ai", "Team AI", "owner-1");
|
||||
setId(namespace, 2L);
|
||||
namespace.setType(NamespaceType.TEAM);
|
||||
|
||||
Skill skill = new Skill(2L, "demo-skill", "owner-1", SkillVisibility.PUBLIC);
|
||||
setId(skill, 1L);
|
||||
skill.setDisplayName("Demo Skill");
|
||||
skill.setStatus(SkillStatus.ACTIVE);
|
||||
skill.setLatestVersionId(10L);
|
||||
|
||||
SkillVersion version = new SkillVersion(1L, "1.0.0", "owner-1");
|
||||
setId(version, 10L);
|
||||
version.setStatus(SkillVersionStatus.PUBLISHED);
|
||||
|
||||
when(namespaceRepository.findBySlug("team-ai")).thenReturn(Optional.of(namespace));
|
||||
when(skillRepository.findByNamespaceIdAndSlug(2L, "demo-skill")).thenReturn(List.of(skill));
|
||||
when(visibilityChecker.canAccess(skill, null, Map.of())).thenReturn(true);
|
||||
when(skillVersionRepository.findBySkillIdAndVersion(1L, "1.0.0")).thenReturn(Optional.of(version));
|
||||
when(objectStorageService.exists("packages/1/10/bundle.zip")).thenReturn(false);
|
||||
when(skillFileRepository.findByVersionId(10L)).thenReturn(List.of(
|
||||
new SkillFile(10L, "SKILL.md", 4L, "text/markdown", "hash", "skills/1/10/SKILL.md")));
|
||||
when(objectStorageService.exists("skills/1/10/SKILL.md")).thenReturn(true);
|
||||
when(objectStorageService.getObject("skills/1/10/SKILL.md")).thenReturn(new ByteArrayInputStream("test".getBytes()));
|
||||
|
||||
assertThrows(DomainForbiddenException.class, () ->
|
||||
service.downloadVersion("team-ai", "demo-skill", "1.0.0", null, Map.of()));
|
||||
SkillDownloadService.DownloadResult result = service.downloadVersion("team-ai", "demo-skill", "1.0.0", null, Map.of());
|
||||
|
||||
verify(visibilityChecker, never()).canAccess(any(), any(), anyMap());
|
||||
verify(skillRepository, never()).incrementDownloadCount(anyLong());
|
||||
verify(skillVersionStatsRepository, never()).incrementDownloadCount(anyLong(), anyLong());
|
||||
verify(eventPublisher, never()).publishEvent(any(SkillDownloadedEvent.class));
|
||||
assertNotNull(result);
|
||||
assertEquals("Demo Skill-1.0.0.zip", result.filename());
|
||||
verify(skillRepository).incrementDownloadCount(1L);
|
||||
verify(skillVersionStatsRepository).incrementDownloadCount(10L, 1L);
|
||||
verify(eventPublisher).publishEvent(any(SkillDownloadedEvent.class));
|
||||
}
|
||||
|
||||
private void setId(Object entity, Long id) throws Exception {
|
||||
|
|
|
|||
Loading…
Add table
Reference in a new issue