From 0ba15558435be05dcbc76252f32e0aa8295e8f4e Mon Sep 17 00:00:00 2001 From: PR Review Helper Date: Fri, 5 Jun 2026 11:05:44 +0800 Subject: [PATCH] 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 --- cli/src/commands/install.ts | 8 +- cli/src/commands/remove.ts | 8 +- cli/src/shared/skill-name-parser.ts | 27 ++++++ .../unit/shared/skill-name-parser.test.ts | 90 +++++++++++++++++++ .../skill/service/SkillDownloadService.java | 8 +- .../service/SkillDownloadServiceTest.java | 26 ++++-- 6 files changed, 151 insertions(+), 16 deletions(-) create mode 100644 cli/src/shared/skill-name-parser.ts create mode 100644 cli/test/unit/shared/skill-name-parser.test.ts diff --git a/cli/src/commands/install.ts b/cli/src/commands/install.ts index e9937a7ba..0feed791b 100644 --- a/cli/src/commands/install.ts +++ b/cli/src/commands/install.ts @@ -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 { @@ -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({ diff --git a/cli/src/commands/remove.ts b/cli/src/commands/remove.ts index 67f47f1d4..4e8543b7c 100644 --- a/cli/src/commands/remove.ts +++ b/cli/src/commands/remove.ts @@ -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 { +export async function removeCommand(skillNameArg: string, options: RemoveCommandOptions): Promise { 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') diff --git a/cli/src/shared/skill-name-parser.ts b/cli/src/shared/skill-name-parser.ts new file mode 100644 index 000000000..05e0662b9 --- /dev/null +++ b/cli/src/shared/skill-name-parser.ts @@ -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) + } +} diff --git a/cli/test/unit/shared/skill-name-parser.test.ts b/cli/test/unit/shared/skill-name-parser.test.ts new file mode 100644 index 000000000..4dc19790d --- /dev/null +++ b/cli/test/unit/shared/skill-name-parser.test.ts @@ -0,0 +1,90 @@ +import { describe, it, expect } from 'bun:test' +import { parseSkillName } from '../../../src/shared/skill-name-parser' + +describe('parseSkillName', () => { + describe('with namespace--slug format', () => { + it('should parse namespace and slug separated by double dash', () => { + const result = parseSkillName('astroclaw--api-gateway') + expect(result).toEqual({ + namespace: 'astroclaw', + slug: 'api-gateway' + }) + }) + + it('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' + }) + }) + + it('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', () => { + it('should use default namespace when no separator present', () => { + const result = parseSkillName('api-gateway') + expect(result).toEqual({ + namespace: 'global', + slug: 'api-gateway' + }) + }) + + it('should use custom default namespace when provided', () => { + const result = parseSkillName('api-gateway', 'myorg') + expect(result).toEqual({ + namespace: 'myorg', + slug: 'api-gateway' + }) + }) + + it('should handle slug with single dashes', () => { + const result = parseSkillName('my-skill-name') + expect(result).toEqual({ + namespace: 'global', + slug: 'my-skill-name' + }) + }) + }) + + describe('edge cases', () => { + it('should handle separator at start', () => { + const result = parseSkillName('--api-gateway') + expect(result).toEqual({ + namespace: 'global', + slug: 'api-gateway' + }) + }) + + it('should handle separator at end', () => { + const result = parseSkillName('astroclaw--') + expect(result).toEqual({ + namespace: 'global', + slug: 'astroclaw' + }) + }) + + it('should handle empty string', () => { + const result = parseSkillName('') + expect(result).toEqual({ + namespace: 'global', + slug: '' + }) + }) + + it('should handle just separator', () => { + const result = parseSkillName('--') + expect(result).toEqual({ + namespace: 'global', + slug: '' + }) + }) + }) +}) diff --git a/server/skillhub-domain/src/main/java/com/iflytek/skillhub/domain/skill/service/SkillDownloadService.java b/server/skillhub-domain/src/main/java/com/iflytek/skillhub/domain/skill/service/SkillDownloadService.java index 3bb194ff3..6a62b749e 100644 --- a/server/skillhub-domain/src/main/java/com/iflytek/skillhub/domain/skill/service/SkillDownloadService.java +++ b/server/skillhub-domain/src/main/java/com/iflytek/skillhub/domain/skill/service/SkillDownloadService.java @@ -4,7 +4,6 @@ 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 @@ private void assertCanDownload(Namespace namespace, Skill skill, String currentUserId, Map 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 @@ private void assertCanDownload(Namespace namespace, } } - 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) { diff --git a/server/skillhub-domain/src/test/java/com/iflytek/skillhub/domain/skill/service/SkillDownloadServiceTest.java b/server/skillhub-domain/src/test/java/com/iflytek/skillhub/domain/skill/service/SkillDownloadServiceTest.java index ba24003b5..4e8251d16 100644 --- a/server/skillhub-domain/src/test/java/com/iflytek/skillhub/domain/skill/service/SkillDownloadServiceTest.java +++ b/server/skillhub-domain/src/test/java/com/iflytek/skillhub/domain/skill/service/SkillDownloadServiceTest.java @@ -318,26 +318,38 @@ void testDownloadVersion_AllowsAnonymousForGlobalPublicSkill() throws Exception } @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 {