Skip to content
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
8 changes: 6 additions & 2 deletions cli/src/commands/install.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -74,7 +75,7 @@ async function defaultPromptScope(): Promise<'user' | 'project'> {
}

export async function installCommand(
slug: string,
skillNameArg: string,
options: InstallCommandOptions,
deps: InstallCommandDeps = {}
): Promise<string> {
Expand All @@ -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({
Expand Down
8 changes: 6 additions & 2 deletions cli/src/commands/remove.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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)
}
Expand All @@ -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')
Expand Down
27 changes: 27 additions & 0 deletions cli/src/shared/skill-name-parser.ts
Original file line number Diff line number Diff line change
@@ -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 changes: 90 additions & 0 deletions cli/test/unit/shared/skill-name-parser.test.ts
Original file line number Diff line number Diff line change
@@ -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: ''
})
})
})
})
Original file line number Diff line number Diff line change
Expand Up @@ -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.*;
Expand Down Expand Up @@ -268,17 +267,16 @@ private void assertCanDownload(Namespace namespace,
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)) {
throw new DomainForbiddenException("error.skill.access.denied", skill.getSlug());
}
}

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) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down
Loading