diff --git a/cli/src/commands/install.ts b/cli/src/commands/install.ts index e9937a7b..0feed791 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 67f47f1d..4e8543b7 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 00000000..05e0662b --- /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 00000000..b86771ce --- /dev/null +++ b/cli/test/unit/shared/skill-name-parser.test.ts @@ -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: '' + }) + }) + }) +}) diff --git a/server/skillhub-app/src/main/java/com/iflytek/skillhub/controller/portal/MeController.java b/server/skillhub-app/src/main/java/com/iflytek/skillhub/controller/portal/MeController.java index f5869eb1..0dd5f2ba 100644 --- a/server/skillhub-app/src/main/java/com/iflytek/skillhub/controller/portal/MeController.java +++ b/server/skillhub-app/src/main/java/com/iflytek/skillhub/controller/portal/MeController.java @@ -34,6 +34,8 @@ public class MeController extends BaseApiController { @RequestParam(defaultValue = "0") int page, @RequestParam(defaultValue = "10") int size, @RequestParam(required = false) String filter, + @RequestParam(required = false) String q, + @RequestParam(required = false) String namespace, @AuthenticationPrincipal PlatformPrincipal principal) { if (principal == null) { throw new UnauthorizedException("error.auth.required"); @@ -41,7 +43,7 @@ public class MeController extends BaseApiController { return ok( "response.success.read", - mySkillAppService.listMySkills(principal.userId(), page, size, filter, principal.platformRoles()) + mySkillAppService.listMySkills(principal.userId(), page, size, filter, q, namespace, principal.platformRoles()) ); } diff --git a/server/skillhub-app/src/main/java/com/iflytek/skillhub/service/MySkillAppService.java b/server/skillhub-app/src/main/java/com/iflytek/skillhub/service/MySkillAppService.java index 9ad9e6f3..e36dc201 100644 --- a/server/skillhub-app/src/main/java/com/iflytek/skillhub/service/MySkillAppService.java +++ b/server/skillhub-app/src/main/java/com/iflytek/skillhub/service/MySkillAppService.java @@ -1,5 +1,7 @@ package com.iflytek.skillhub.service; +import com.iflytek.skillhub.domain.namespace.Namespace; +import com.iflytek.skillhub.domain.namespace.NamespaceRepository; import com.iflytek.skillhub.domain.skill.Skill; import com.iflytek.skillhub.domain.skill.SkillRepository; import com.iflytek.skillhub.domain.skill.SkillVersionRepository; @@ -36,6 +38,7 @@ public class MySkillAppService { private final SkillSubscriptionRepository skillSubscriptionRepository; private final MySkillQueryRepository mySkillQueryRepository; private final SkillLifecycleProjectionService skillLifecycleProjectionService; + private final NamespaceRepository namespaceRepository; public MySkillAppService( SkillRepository skillRepository, @@ -43,17 +46,19 @@ public class MySkillAppService { SkillStarRepository skillStarRepository, SkillSubscriptionRepository skillSubscriptionRepository, MySkillQueryRepository mySkillQueryRepository, - SkillLifecycleProjectionService skillLifecycleProjectionService) { + SkillLifecycleProjectionService skillLifecycleProjectionService, + NamespaceRepository namespaceRepository) { this.skillRepository = skillRepository; this.skillVersionRepository = skillVersionRepository; this.skillStarRepository = skillStarRepository; this.skillSubscriptionRepository = skillSubscriptionRepository; this.mySkillQueryRepository = mySkillQueryRepository; this.skillLifecycleProjectionService = skillLifecycleProjectionService; + this.namespaceRepository = namespaceRepository; } public PageResponse listMySkills(String userId, int page, int size) { - return listMySkills(userId, page, size, null, java.util.Set.of()); + return listMySkills(userId, page, size, null, null, null, java.util.Set.of()); } public PageResponse listMySkills(String userId, @@ -61,10 +66,27 @@ public class MySkillAppService { int size, String filter, java.util.Set platformRoles) { + return listMySkills(userId, page, size, filter, null, null, platformRoles); + } + + public PageResponse listMySkills(String userId, + int page, + int size, + String filter, + String keyword, + String namespace, + java.util.Set platformRoles) { MySkillFilter normalizedFilter = parseFilter(filter); - Page skillPage = normalizedFilter == MySkillFilter.ALL - ? skillRepository.findByOwnerId(userId, PageRequest.of(page, size)) - : filterSkillsByLifecycle(userId, page, size, normalizedFilter, platformRoles); + + Page skillPage; + if (normalizedFilter == MySkillFilter.ALL + && (keyword == null || keyword.isBlank()) + && (namespace == null || namespace.isBlank())) { + skillPage = skillRepository.findByOwnerId(userId, PageRequest.of(page, size)); + } else { + skillPage = filterSkills(userId, page, size, normalizedFilter, keyword, namespace, platformRoles); + } + List items = mySkillQueryRepository.getSkillSummaries(skillPage.getContent(), userId); return new PageResponse<>(items, skillPage.getTotalElements(), skillPage.getNumber(), skillPage.getSize()); @@ -118,15 +140,34 @@ public class MySkillAppService { return new PageResponse<>(items, subPage.getTotalElements(), subPage.getNumber(), subPage.getSize()); } - private Page filterSkillsByLifecycle(String userId, - int page, - int size, - MySkillFilter filter, - java.util.Set platformRoles) { + private Page filterSkills(String userId, + int page, + int size, + MySkillFilter filter, + String keyword, + String namespace, + java.util.Set platformRoles) { List skills = skillRepository.findByOwnerId(userId); + + // Namespace filter + Long namespaceId = null; + if (namespace != null && !namespace.isBlank()) { + namespaceId = namespaceRepository.findBySlug(namespace.trim()) + .map(Namespace::getId) + .orElse(-1L); + } + + final Long finalNamespaceId = namespaceId; + String normalizedKeyword = keyword != null && !keyword.isBlank() + ? keyword.trim().toLowerCase(java.util.Locale.ROOT) + : null; + List filtered = skills.stream() + .filter(skill -> matchesNamespace(skill, finalNamespaceId)) + .filter(skill -> matchesKeyword(skill, normalizedKeyword)) .filter(skill -> matchesFilter(skill, filter, platformRoles)) .toList(); + int fromIndex = Math.min(page * size, filtered.size()); int toIndex = Math.min(fromIndex + size, filtered.size()); return new PageImpl<>( @@ -136,6 +177,35 @@ public class MySkillAppService { ); } + private boolean matchesNamespace(Skill skill, Long namespaceId) { + if (namespaceId == null) { + return true; + } + if (namespaceId == -1L) { + return false; + } + return skill.getNamespaceId().equals(namespaceId); + } + + private boolean matchesKeyword(Skill skill, String keyword) { + if (keyword == null) { + return true; + } + String displayName = skill.getDisplayName() != null ? skill.getDisplayName().toLowerCase(java.util.Locale.ROOT) : ""; + String slug = skill.getSlug() != null ? skill.getSlug().toLowerCase(java.util.Locale.ROOT) : ""; + String summary = skill.getSummary() != null ? skill.getSummary().toLowerCase(java.util.Locale.ROOT) : ""; + + return displayName.contains(keyword) || slug.contains(keyword) || summary.contains(keyword); + } + + private Page filterSkillsByLifecycle(String userId, + int page, + int size, + MySkillFilter filter, + java.util.Set platformRoles) { + return filterSkills(userId, page, size, filter, null, null, platformRoles); + } + private boolean matchesFilter(Skill skill, MySkillFilter filter, java.util.Set platformRoles) { if (filter == MySkillFilter.HIDDEN) { return platformRoles.contains("SUPER_ADMIN") && skill.isHidden(); diff --git a/server/skillhub-app/src/main/java/com/iflytek/skillhub/task/IdempotencyCleanupTask.java b/server/skillhub-app/src/main/java/com/iflytek/skillhub/task/IdempotencyCleanupTask.java index ea40dfee..c5b320c3 100644 --- a/server/skillhub-app/src/main/java/com/iflytek/skillhub/task/IdempotencyCleanupTask.java +++ b/server/skillhub-app/src/main/java/com/iflytek/skillhub/task/IdempotencyCleanupTask.java @@ -42,7 +42,7 @@ public class IdempotencyCleanupTask { Instant threshold = Instant.now(clock).minusSeconds(STALE_THRESHOLD_MINUTES * 60); int updated = idempotencyRecordRepository.markStaleAsFailed(threshold); if (updated > 0) { - logger.info("Marked {} stale processing records as failed", updated); + logger.info("Marked {} stale processing records as failed before threshold={}", updated, threshold); } } } diff --git a/server/skillhub-app/src/test/java/com/iflytek/skillhub/controller/MeControllerTest.java b/server/skillhub-app/src/test/java/com/iflytek/skillhub/controller/MeControllerTest.java index af16f4dd..e4e7b80b 100644 --- a/server/skillhub-app/src/test/java/com/iflytek/skillhub/controller/MeControllerTest.java +++ b/server/skillhub-app/src/test/java/com/iflytek/skillhub/controller/MeControllerTest.java @@ -56,7 +56,7 @@ class MeControllerTest { principal, null, List.of(new SimpleGrantedAuthority("ROLE_USER")) ); - given(mySkillAppService.listMySkills("user-42", 1, 5, null, Set.of("USER"))) + given(mySkillAppService.listMySkills("user-42", 1, 5, null, null, null, Set.of("USER"))) .willReturn(new PageResponse<>( List.of(new SkillSummaryResponse( 7L, @@ -103,7 +103,7 @@ class MeControllerTest { principal, null, List.of(new SimpleGrantedAuthority("ROLE_SUPER_ADMIN")) ); - given(mySkillAppService.listMySkills("user-42", 0, 10, "HIDDEN", Set.of("SUPER_ADMIN"))) + given(mySkillAppService.listMySkills("user-42", 0, 10, "HIDDEN", null, null, Set.of("SUPER_ADMIN"))) .willReturn(new PageResponse<>(List.of(), 0, 0, 10)); mockMvc.perform(get("/api/v1/me/skills") diff --git a/server/skillhub-app/src/test/java/com/iflytek/skillhub/service/MySkillAppServiceTest.java b/server/skillhub-app/src/test/java/com/iflytek/skillhub/service/MySkillAppServiceTest.java index c4faf136..7c8bbb3e 100644 --- a/server/skillhub-app/src/test/java/com/iflytek/skillhub/service/MySkillAppServiceTest.java +++ b/server/skillhub-app/src/test/java/com/iflytek/skillhub/service/MySkillAppServiceTest.java @@ -75,7 +75,8 @@ class MySkillAppServiceTest { skillStarRepository, skillSubscriptionRepository, mySkillQueryRepository, - skillLifecycleProjectionService + skillLifecycleProjectionService, + namespaceRepository ); } @@ -265,6 +266,104 @@ class MySkillAppServiceTest { assertThat(result.items().get(0).headlineVersion().status()).isEqualTo("REJECTED"); } + @Test + void listMySkills_hidesStaleRejectedVersionOlderThanPublished() { + Skill skill = createSkill(6L, 101L, "recovered-skill", "user-1"); + SkillVersion rejectedVersion = createVersion(6L, 60L, "1.0.0", SkillVersionStatus.REJECTED, "2026-03-15T09:30:00Z"); + SkillVersion publishedVersion = createVersion(6L, 61L, "2.0.0", SkillVersionStatus.PUBLISHED, "2026-03-16T09:30:00Z"); + + given(skillRepository.findByOwnerId("user-1", PageRequest.of(0, 10))) + .willReturn(new PageImpl<>(List.of(skill), PageRequest.of(0, 10), 1)); + given(skillVersionRepository.findBySkillIdAndStatus(6L, SkillVersionStatus.PUBLISHED)).willReturn(List.of(publishedVersion)); + given(skillVersionRepository.findBySkillId(6L)).willReturn(List.of(rejectedVersion, publishedVersion)); + given(namespaceRepository.findByIdIn(List.of(101L))).willReturn(List.of(namespace(101L, "team-ai"))); + + var result = service.listMySkills("user-1", 0, 10); + + assertThat(result.items()).hasSize(1); + assertThat(result.items().get(0).headlineVersion().status()).isEqualTo("PUBLISHED"); + assertThat(result.items().get(0).headlineVersion().version()).isEqualTo("2.0.0"); + assertThat(result.items().get(0).ownerPreviewVersion()).isNull(); + } + + @Test + void listMySkills_filtersByKeywordAcrossDisplayNameSlugAndSummary() { + Skill alpha = createSkill(1L, 101L, "alpha-tool", "user-1"); + alpha.setDisplayName("Alpha Assistant"); + Skill beta = createSkill(2L, 101L, "beta-tool", "user-1"); + beta.setDisplayName("Beta Tool"); + beta.setSummary("This tool helps with alpha testing"); + Skill gamma = createSkill(3L, 101L, "gamma-tool", "user-1"); + gamma.setDisplayName("Gamma Service"); + SkillVersion publishedVersion = createVersion(1L, 10L, "1.0.0", SkillVersionStatus.PUBLISHED, "2026-03-15T09:30:00Z"); + + given(skillRepository.findByOwnerId("user-1")).willReturn(List.of(alpha, beta, gamma)); + given(skillVersionRepository.findBySkillId(1L)).willReturn(List.of(publishedVersion)); + given(skillVersionRepository.findBySkillId(2L)).willReturn(List.of()); + given(namespaceRepository.findByIdIn(List.of(101L))).willReturn(List.of(namespace(101L, "team-ai"))); + + var result = service.listMySkills("user-1", 0, 10, null, "alpha", null, Set.of("USER")); + + assertThat(result.total()).isEqualTo(2); + assertThat(result.items()).extracting("slug") + .containsExactlyInAnyOrder("alpha-tool", "beta-tool"); + } + + @Test + void listMySkills_filtersByNamespaceSlug() { + Skill aiSkill = createSkill(1L, 101L, "ai-tool", "user-1"); + Skill mlSkill = createSkill(2L, 102L, "ml-tool", "user-1"); + SkillVersion v1 = createVersion(1L, 10L, "1.0.0", SkillVersionStatus.PUBLISHED, "2026-03-15T09:30:00Z"); + + given(skillRepository.findByOwnerId("user-1")).willReturn(List.of(aiSkill, mlSkill)); + given(skillVersionRepository.findBySkillId(1L)).willReturn(List.of(v1)); + given(namespaceRepository.findBySlug("team-ai")).willReturn(java.util.Optional.of(namespace(101L, "team-ai"))); + given(namespaceRepository.findByIdIn(List.of(101L))).willReturn(List.of(namespace(101L, "team-ai"))); + + var result = service.listMySkills("user-1", 0, 10, null, null, "team-ai", Set.of("USER")); + + assertThat(result.total()).isEqualTo(1); + assertThat(result.items()).extracting("slug").containsExactly("ai-tool"); + } + + @Test + void listMySkills_returnsEmptyWhenNamespaceSlugNotFound() { + Skill skill = createSkill(1L, 101L, "ai-tool", "user-1"); + + given(skillRepository.findByOwnerId("user-1")).willReturn(List.of(skill)); + given(namespaceRepository.findBySlug("missing-namespace")).willReturn(java.util.Optional.empty()); + + var result = service.listMySkills("user-1", 0, 10, null, null, "missing-namespace", Set.of("USER")); + + assertThat(result.total()).isZero(); + assertThat(result.items()).isEmpty(); + } + + @Test + void listMySkills_combinesKeywordNamespaceAndStatusFilters() { + Skill aiAlpha = createSkill(1L, 101L, "ai-alpha", "user-1"); + aiAlpha.setDisplayName("AI Alpha"); + Skill aiBeta = createSkill(2L, 101L, "ai-beta", "user-1"); + aiBeta.setDisplayName("AI Beta"); + Skill mlAlpha = createSkill(3L, 102L, "ml-alpha", "user-1"); + mlAlpha.setDisplayName("ML Alpha"); + SkillVersion v1 = createVersion(1L, 10L, "1.0.0", SkillVersionStatus.PUBLISHED, "2026-03-15T09:30:00Z"); + SkillVersion v2 = createVersion(2L, 20L, "1.0.0", SkillVersionStatus.REJECTED, "2026-03-15T09:30:00Z"); + SkillVersion v3 = createVersion(3L, 30L, "1.0.0", SkillVersionStatus.PUBLISHED, "2026-03-15T09:30:00Z"); + + given(skillRepository.findByOwnerId("user-1")).willReturn(List.of(aiAlpha, aiBeta, mlAlpha)); + given(skillVersionRepository.findBySkillIdAndStatus(1L, SkillVersionStatus.PUBLISHED)).willReturn(List.of(v1)); + given(skillVersionRepository.findBySkillId(1L)).willReturn(List.of(v1)); + given(namespaceRepository.findBySlug("team-ai")).willReturn(java.util.Optional.of(namespace(101L, "team-ai"))); + given(namespaceRepository.findByIdIn(List.of(101L))).willReturn(List.of(namespace(101L, "team-ai"))); + + var result = service.listMySkills("user-1", 0, 10, "PUBLISHED", "alpha", "team-ai", Set.of("USER")); + + assertThat(result.total()).isEqualTo(1); + assertThat(result.items()).extracting("slug").containsExactly("ai-alpha"); + } + + private Skill createSkill(Long id, Long namespaceId, String slug, String ownerId) { Skill skill = new Skill(namespaceId, slug, ownerId, SkillVisibility.PUBLIC); skill.setDisplayName(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 3bb194ff..6a62b749 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.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 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) { diff --git a/server/skillhub-domain/src/main/java/com/iflytek/skillhub/domain/skill/service/SkillGovernanceService.java b/server/skillhub-domain/src/main/java/com/iflytek/skillhub/domain/skill/service/SkillGovernanceService.java index 820bbba9..bbb748f8 100644 --- a/server/skillhub-domain/src/main/java/com/iflytek/skillhub/domain/skill/service/SkillGovernanceService.java +++ b/server/skillhub-domain/src/main/java/com/iflytek/skillhub/domain/skill/service/SkillGovernanceService.java @@ -182,12 +182,15 @@ public class SkillGovernanceService { deleteStorageAfterCommit(skill, namespaceSlug, storageKeys); skillFileRepository.deleteByVersionId(version.getId()); securityScanService.softDeleteByVersionId(version.getId()); - skillVersionRepository.delete(version); + // FK 约束 fk_skill_latest_version 阻止删除 skill_version 当 skill.latest_version_id 还指向它。 + // 必须先解开引用并 flush,让 PG 在 delete 时看不到引用。 if (version.getId().equals(skill.getLatestVersionId())) { skill.setLatestVersionId(findLatestPublishedVersionId(skill.getId())); skill.setUpdatedBy(actorUserId); skillRepository.save(skill); + skillRepository.flush(); } + skillVersionRepository.delete(version); auditLogService.record( actorUserId, "DELETE_SKILL_VERSION", diff --git a/server/skillhub-domain/src/main/java/com/iflytek/skillhub/domain/skill/service/SkillLifecycleProjectionService.java b/server/skillhub-domain/src/main/java/com/iflytek/skillhub/domain/skill/service/SkillLifecycleProjectionService.java index 711ff60d..368ae850 100644 --- a/server/skillhub-domain/src/main/java/com/iflytek/skillhub/domain/skill/service/SkillLifecycleProjectionService.java +++ b/server/skillhub-domain/src/main/java/com/iflytek/skillhub/domain/skill/service/SkillLifecycleProjectionService.java @@ -39,6 +39,10 @@ public class SkillLifecycleProjectionService { ResolutionMode resolutionMode ) {} + private static final Comparator RECENCY = Comparator + .comparing(SkillVersion::getCreatedAt, Comparator.nullsLast(Comparator.naturalOrder())) + .thenComparing(SkillVersion::getId, Comparator.nullsLast(Comparator.naturalOrder())); + private final SkillVersionRepository skillVersionRepository; public SkillLifecycleProjectionService(SkillVersionRepository skillVersionRepository) { @@ -46,22 +50,26 @@ public class SkillLifecycleProjectionService { } public Projection projectForViewer(Skill skill, String currentUserId, Map userNsRoles) { - VersionProjection publishedVersion = toProjection(resolvePublishedVersion(skill)); - VersionProjection ownerPreviewVersion = toProjection(resolveOwnerPendingPreview(skill, currentUserId, userNsRoles)); - VersionProjection headlineVersion = publishedVersion != null ? publishedVersion : ownerPreviewVersion; - ResolutionMode resolutionMode = headlineVersion == null - ? ResolutionMode.NONE - : publishedVersion != null ? ResolutionMode.PUBLISHED : ResolutionMode.OWNER_PREVIEW; - return new Projection(headlineVersion, publishedVersion, ownerPreviewVersion, resolutionMode); + SkillVersion published = resolvePublishedVersion(skill); + SkillVersion preview = canManage(skill, currentUserId, userNsRoles) + ? resolveNewerNonPublishedVersion(skill, published) + : null; + return buildProjection(published, preview); } public Projection projectForOwnerSummary(Skill skill) { - VersionProjection publishedVersion = toProjection(resolvePublishedVersion(skill)); - VersionProjection ownerPreviewVersion = toProjection(resolveNewestNonPublishedVersion(skill)); + SkillVersion published = resolvePublishedVersion(skill); + SkillVersion preview = resolveNewerNonPublishedVersion(skill, published); + return buildProjection(published, preview); + } + + private Projection buildProjection(SkillVersion published, SkillVersion preview) { + VersionProjection publishedVersion = toProjection(published); + VersionProjection ownerPreviewVersion = toProjection(preview); VersionProjection headlineVersion = publishedVersion != null ? publishedVersion : ownerPreviewVersion; - ResolutionMode resolutionMode = headlineVersion == null - ? ResolutionMode.NONE - : publishedVersion != null ? ResolutionMode.PUBLISHED : ResolutionMode.OWNER_PREVIEW; + ResolutionMode resolutionMode = headlineVersion == null ? ResolutionMode.NONE + : publishedVersion != null ? ResolutionMode.PUBLISHED + : ResolutionMode.OWNER_PREVIEW; return new Projection(headlineVersion, publishedVersion, ownerPreviewVersion, resolutionMode); } @@ -116,27 +124,19 @@ public class SkillLifecycleProjectionService { } /** - * Returns the newest non-published version the owner can preview. - * Includes PENDING_REVIEW, REJECTED, DRAFT, SCANNING, SCAN_FAILED — any status - * that isn't already covered by the published projection and isn't yanked. + * Returns the newest non-published version (PENDING_REVIEW, REJECTED, DRAFT, SCANNING, + * SCAN_FAILED) that represents a NEW round of work layered on top of the current published + * version. A non-published version that is older than the published version is treated as + * settled history (e.g. an early rejected attempt later superseded by a published release) + * and is intentionally not surfaced, so the owner does not see a stale preview/rejected badge + * next to an already-published skill. */ - private SkillVersion resolveOwnerPendingPreview(Skill skill, String currentUserId, Map userNsRoles) { - if (!canManage(skill, currentUserId, userNsRoles)) { - return null; - } + private SkillVersion resolveNewerNonPublishedVersion(Skill skill, SkillVersion publishedVersion) { return skillVersionRepository.findBySkillId(skill.getId()).stream() - .filter(v -> v.getStatus() != SkillVersionStatus.PUBLISHED - && v.getStatus() != SkillVersionStatus.YANKED) - .max(versionComparator()) - .orElse(null); - } - - private SkillVersion resolveNewestNonPublishedVersion(Skill skill) { - List versions = skillVersionRepository.findBySkillId(skill.getId()); - return versions.stream() .filter(version -> version.getStatus() != SkillVersionStatus.PUBLISHED && version.getStatus() != SkillVersionStatus.YANKED) - .max(versionComparator()) + .filter(version -> publishedVersion == null || RECENCY.compare(version, publishedVersion) > 0) + .max(RECENCY) .orElse(null); } 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 ba24003b..4e8251d1 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 @@ 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 { diff --git a/web/e2e/helpers/test-data-builder.ts b/web/e2e/helpers/test-data-builder.ts index f2cb47d6..a6b77adc 100644 --- a/web/e2e/helpers/test-data-builder.ts +++ b/web/e2e/helpers/test-data-builder.ts @@ -1,4 +1,4 @@ -import { mkdtempSync, readFileSync, rmSync, writeFileSync } from 'node:fs' +import { mkdirSync, mkdtempSync, readFileSync, rmSync, writeFileSync } from 'node:fs' import { tmpdir } from 'node:os' import { execFileSync } from 'node:child_process' import path from 'node:path' @@ -65,6 +65,11 @@ export interface SeedSkillOptions { description?: string version?: string readmeHeading?: string + readmeBody?: string + extraFiles?: Array<{ + path: string + content: string + }> } function asApiErrorBody(value: unknown): string { @@ -130,8 +135,13 @@ function buildSkillPackageZipBuffer(suffix: string, options?: SeedSkillOptions): execFileSync('mkdir', ['-p', packageDir]) writeFileSync(path.join(packageDir, 'SKILL.md'), skillMd, 'utf8') - writeFileSync(path.join(packageDir, 'README.md'), `# ${readmeHeading}\n`, 'utf8') - execFileSync('zip', ['-q', '-r', zipPath, 'SKILL.md', 'README.md'], { cwd: packageDir }) + writeFileSync(path.join(packageDir, 'README.md'), options?.readmeBody ?? `# ${readmeHeading}\n`, 'utf8') + for (const extraFile of options?.extraFiles ?? []) { + const targetPath = path.join(packageDir, extraFile.path) + mkdirSync(path.dirname(targetPath), { recursive: true }) + writeFileSync(targetPath, extraFile.content, 'utf8') + } + execFileSync('zip', ['-q', '-r', zipPath, '.'], { cwd: packageDir }) return readFileSync(zipPath) } finally { rmSync(tempRoot, { recursive: true, force: true }) @@ -146,8 +156,13 @@ function createSkillPackageZipFile(suffix: string, options?: SeedSkillOptions): execFileSync('mkdir', ['-p', packageDir]) writeFileSync(path.join(packageDir, 'SKILL.md'), skillMd, 'utf8') - writeFileSync(path.join(packageDir, 'README.md'), `# ${readmeHeading}\n`, 'utf8') - execFileSync('zip', ['-q', '-r', zipPath, 'SKILL.md', 'README.md'], { cwd: packageDir }) + writeFileSync(path.join(packageDir, 'README.md'), options?.readmeBody ?? `# ${readmeHeading}\n`, 'utf8') + for (const extraFile of options?.extraFiles ?? []) { + const targetPath = path.join(packageDir, extraFile.path) + mkdirSync(path.dirname(targetPath), { recursive: true }) + writeFileSync(targetPath, extraFile.content, 'utf8') + } + execFileSync('zip', ['-q', '-r', zipPath, '.'], { cwd: packageDir }) return { filePath: zipPath, diff --git a/web/e2e/public-skill-detail-anonymous.spec.ts b/web/e2e/public-skill-detail-anonymous.spec.ts index 56ba8885..720c2796 100644 --- a/web/e2e/public-skill-detail-anonymous.spec.ts +++ b/web/e2e/public-skill-detail-anonymous.spec.ts @@ -11,6 +11,10 @@ function latestSeed(seed: PreparedSearchSeed) { } } +function escapeRegExp(value: string) { + return value.replace(/[.*+?^${}()|[\]\\]/g, '\\$&') +} + let seeded: PreparedSearchSeed | undefined test.describe('Public Skill Detail Anonymous Access (Real API)', () => { @@ -36,11 +40,25 @@ test.describe('Public Skill Detail Anonymous Access (Real API)', () => { await card.click() - await expect(page).toHaveURL(new RegExp(`/space/${current.skill.namespace}/${current.skill.slug}$`)) + await expect(page).toHaveURL(new RegExp(`/space/${current.skill.namespace}/${current.skill.slug}(\\?|$)`)) await expect(page).not.toHaveURL(/\/login\?returnTo=/) await expect(page.getByRole('heading', { name: current.skillName, exact: true })).toBeVisible() await expect(page.getByText('Install', { exact: true })).toBeVisible() - await expect(page.getByText(new RegExp(`npx clawhub install ${current.skill.slug}`))).toBeVisible() + const clawhubTarget = current.skill.namespace === 'global' + ? current.skill.slug + : `${current.skill.namespace}--${current.skill.slug}` + const skillhubNamespace = current.skill.namespace === 'global' + ? '' + : ` --namespace ${current.skill.namespace}` + + await expect(page.getByRole('tab', { name: 'ClawHub CLI' })).toHaveAttribute('aria-selected', 'true') + await expect(page.getByText(new RegExp(`npx clawhub install ${escapeRegExp(clawhubTarget)} --registry`))).toBeVisible() + await expect(page.getByRole('tab', { name: 'SkillHub CLI' })).toBeVisible() + + await page.getByRole('tab', { name: 'SkillHub CLI' }).click() + + await expect(page.getByRole('tab', { name: 'SkillHub CLI' })).toHaveAttribute('aria-selected', 'true') + await expect(page.getByText(new RegExp(`npx @astron-team/skillhub@latest install ${escapeRegExp(current.skill.slug)}${escapeRegExp(skillhubNamespace)} --registry`))).toBeVisible() await expect(page.getByRole('button', { name: 'Copy' }).first()).toBeVisible() }) }) diff --git a/web/e2e/reviews-pagination.spec.ts b/web/e2e/reviews-pagination.spec.ts index 2d18b30c..f9906a35 100644 --- a/web/e2e/reviews-pagination.spec.ts +++ b/web/e2e/reviews-pagination.spec.ts @@ -41,6 +41,7 @@ test.describe('Review Management Pagination (Real API)', () => { await page.goto('/dashboard/reviews') await expect(page.getByRole('heading', { name: 'Review Center' })).toBeVisible() + await expect(page.getByRole('tab', { name: 'Skill Reviews' })).toBeVisible() const tabMeta: Record = { PENDING: { tabLabel: 'Pending', summaryPrefix: 'Total' }, @@ -49,7 +50,7 @@ test.describe('Review Management Pagination (Real API)', () => { } for (const status of statuses) { - await page.getByRole('button', { name: tabMeta[status].tabLabel }).click() + await page.getByRole('tab', { name: tabMeta[status].tabLabel }).click() const meta = metaByStatus.get(status) if (!meta) { diff --git a/web/e2e/skill-detail-relative-links.spec.ts b/web/e2e/skill-detail-relative-links.spec.ts new file mode 100644 index 00000000..5f1f353c --- /dev/null +++ b/web/e2e/skill-detail-relative-links.spec.ts @@ -0,0 +1,55 @@ +import { expect, test } from '@playwright/test' +import { setEnglishLocale } from './helpers/auth-fixtures' +import { registerSession } from './helpers/session' +import { E2eTestDataBuilder } from './helpers/test-data-builder' + +test.describe('Skill Detail Relative Links (Real API)', () => { + test.beforeEach(async ({ page }, testInfo) => { + await setEnglishLocale(page) + await registerSession(page, testInfo) + }) + + test('previews package files from overview relative links and reports missing files', async ({ page }, testInfo) => { + const builder = new E2eTestDataBuilder(page, testInfo) + await builder.init() + + try { + const namespace = await builder.ensureWritableNamespace() + const skillName = `relative-links-${Date.now().toString(36)}` + const skill = await builder.publishSkill(namespace.slug, { + name: skillName, + readmeBody: [ + `# ${skillName}`, + '', + '[Usage](docs/usage.md)', + '', + '[Missing](docs/missing.md)', + ].join('\n'), + extraFiles: [ + { + path: 'docs/usage.md', + content: '# Usage\n\nThis is linked documentation.', + }, + ], + }) + + await page.goto(`/space/${encodeURIComponent(namespace.slug)}/${encodeURIComponent(skill.slug)}`) + + await expect(page).toHaveURL(new RegExp(`/space/${namespace.slug}/${skill.slug}$`)) + await expect(page.getByRole('link', { name: 'Usage' })).toBeVisible() + await page.getByRole('link', { name: 'Usage' }).click() + await expect(page.getByRole('dialog')).toContainText('usage.md') + await expect(page.getByRole('dialog')).toContainText('This is linked documentation.') + + await page.getByRole('button', { name: 'Close' }).click() + await expect(page.getByRole('dialog')).toBeHidden() + + await page.getByRole('link', { name: 'Missing' }).click() + await expect(page).toHaveURL(new RegExp(`/space/${namespace.slug}/${skill.slug}$`)) + await expect(page.getByText('File not found')).toBeVisible() + await expect(page.getByText('not included in the current skill version')).toBeVisible() + } finally { + await builder.cleanup() + } + }) +}) diff --git a/web/e2e/skill-subscription.spec.ts b/web/e2e/skill-subscription.spec.ts index e14cda1d..c6d74d24 100644 --- a/web/e2e/skill-subscription.spec.ts +++ b/web/e2e/skill-subscription.spec.ts @@ -45,31 +45,14 @@ test.describe('Skill Subscription (Real API)', () => { const subscribeButton = page.getByRole('button', { name: /Subscribe/ }) await expect(subscribeButton).toBeVisible() - const initialCount = await subscribeButton.textContent() - const initialCountMatch = initialCount?.match(/\((\d+)\)/) - const initialCountValue = initialCountMatch ? Number.parseInt(initialCountMatch[1], 10) : 0 - await subscribeButton.click() await expect(page.getByRole('button', { name: /Subscribed/ })).toBeVisible() const subscribedButton = page.getByRole('button', { name: /Subscribed/ }) - const subscribedCount = await subscribedButton.textContent() - const subscribedCountMatch = subscribedCount?.match(/\((\d+)\)/) - const subscribedCountValue = subscribedCountMatch ? Number.parseInt(subscribedCountMatch[1], 10) : 0 - - expect(subscribedCountValue).toBe(initialCountValue + 1) - await subscribedButton.click() await expect(page.getByRole('button', { name: /Subscribe/ })).toBeVisible() - - const unsubscribedButton = page.getByRole('button', { name: /Subscribe/ }) - const unsubscribedCount = await unsubscribedButton.textContent() - const unsubscribedCountMatch = unsubscribedCount?.match(/\((\d+)\)/) - const unsubscribedCountValue = unsubscribedCountMatch ? Number.parseInt(unsubscribedCountMatch[1], 10) : 0 - - expect(unsubscribedCountValue).toBe(initialCountValue) } finally { await adminBuilder.cleanup() await adminContext.close() diff --git a/web/src/api/client.ts b/web/src/api/client.ts index 3204d56a..16d2e7fe 100644 --- a/web/src/api/client.ts +++ b/web/src/api/client.ts @@ -1024,13 +1024,19 @@ export const governanceApi = { } export const meApi = { - async getSkills(params?: { page?: number; size?: number; filter?: string }): Promise<{ items: SkillSummary[]; total: number; page: number; size: number }> { + async getSkills(params?: { page?: number; size?: number; filter?: string; q?: string; namespace?: string }): Promise<{ items: SkillSummary[]; total: number; page: number; size: number }> { const searchParams = new URLSearchParams() searchParams.set('page', String(params?.page ?? 0)) searchParams.set('size', String(params?.size ?? 10)) if (params?.filter) { searchParams.set('filter', params.filter) } + if (params?.q) { + searchParams.set('q', params.q) + } + if (params?.namespace) { + searchParams.set('namespace', params.namespace) + } return fetchJson<{ items: SkillSummary[]; total: number; page: number; size: number }>(`${WEB_API_PREFIX}/me/skills?${searchParams.toString()}`) }, diff --git a/web/src/app/router.tsx b/web/src/app/router.tsx index 8cabf0b3..b053ca79 100644 --- a/web/src/app/router.tsx +++ b/web/src/app/router.tsx @@ -253,6 +253,12 @@ const dashboardSkillsRoute = createRoute({ getParentRoute: () => rootRoute, path: 'dashboard/skills', beforeLoad: requireAuth, + validateSearch: (search: Record): { page?: number; q?: string; namespace?: string; filter?: string } => ({ + page: typeof search.page === 'number' ? search.page : undefined, + q: typeof search.q === 'string' && search.q ? search.q : undefined, + namespace: typeof search.namespace === 'string' && search.namespace ? search.namespace : undefined, + filter: typeof search.filter === 'string' && search.filter ? search.filter : undefined, + }), component: MySkillsPage, }) diff --git a/web/src/features/skill/install-command.test.ts b/web/src/features/skill/install-command.test.ts index 1f4daf50..60b8ee3a 100644 --- a/web/src/features/skill/install-command.test.ts +++ b/web/src/features/skill/install-command.test.ts @@ -1,7 +1,13 @@ import { createElement } from 'react' import { renderToStaticMarkup } from 'react-dom/server' import { afterEach, describe, expect, it, vi } from 'vitest' -import { InstallCommand, buildInstallCommand, buildInstallTarget, getBaseUrl } from './install-command' +import { + InstallCommand, + buildInstallCommand, + buildInstallTarget, + buildSkillhubInstallCommand, + getBaseUrl, +} from './install-command' vi.mock('react-i18next', () => ({ useTranslation: () => ({ @@ -62,6 +68,18 @@ describe('install-command', () => { ) }) + it('builds a one-line SkillHub npx command for the global namespace', () => { + expect(buildSkillhubInstallCommand('global', 'my-skill', 'https://skill.xfyun.cn')).toBe( + 'npx @astron-team/skillhub@latest install my-skill --registry https://skill.xfyun.cn', + ) + }) + + it('builds a one-line SkillHub npx command with namespace for team skills', () => { + expect(buildSkillhubInstallCommand('team-alpha', 'my-skill', 'https://skill.xfyun.cn')).toBe( + 'npx @astron-team/skillhub@latest install my-skill --namespace team-alpha --registry https://skill.xfyun.cn', + ) + }) + it('uses the runtime app base url when available', () => { setMockWindow('https://app.example.com') @@ -92,4 +110,33 @@ describe('install-command', () => { expect(html).toContain('leading-relaxed') expect(html).toContain('break-all') }) + + it('renders install method tabs with only a short active underline', () => { + setMockWindow('https://app.example.com') + + const html = renderToStaticMarkup(createElement(InstallCommand, { + namespace: 'global', + slug: 'meeting-minutes-generator', + })) + + expect(html).toContain('after:w-6') + expect(html).toContain('after:h-0.5') + expect(html).not.toContain('rounded-lg border bg-background/80 p-1') + expect(html).not.toContain('flex-1 rounded-md') + }) + + it('renders ClawHub CLI as the default install method', () => { + setMockWindow('https://app.example.com') + + const html = renderToStaticMarkup(createElement(InstallCommand, { + namespace: 'team-alpha', + slug: 'meeting-minutes-generator', + })) + + expect(html).toContain('skillDetail.installMethodClawhub') + expect(html).toContain('skillDetail.installMethodSkillhub') + expect(html).toContain('aria-selected="true"') + expect(html).toContain('npx clawhub install team-alpha--meeting-minutes-generator --registry https://app.example.com') + expect(html).not.toContain('npx @astron-team/skillhub@latest install meeting-minutes-generator --namespace team-alpha --registry https://app.example.com') + }) }) diff --git a/web/src/features/skill/install-command.tsx b/web/src/features/skill/install-command.tsx index f9989abb..3f409b9d 100644 --- a/web/src/features/skill/install-command.tsx +++ b/web/src/features/skill/install-command.tsx @@ -2,6 +2,7 @@ import { useMemo } from 'react' import { useTranslation } from 'react-i18next' import { Check, Copy } from 'lucide-react' import { Button } from '@/shared/ui/button' +import { Tabs, TabsContent, TabsList, TabsTrigger } from '@/shared/ui/tabs' import { useCopyToClipboard } from '@/shared/lib/clipboard' interface InstallCommandProps { @@ -33,14 +34,22 @@ export function buildInstallCommand(namespace: string, slug: string, baseUrl: st return `npx clawhub install ${installTarget} --registry ${baseUrl}` } -export function InstallCommand({ namespace, slug }: InstallCommandProps) { +export function buildSkillhubInstallCommand(namespace: string, slug: string, baseUrl: string): string { + const namespaceArg = namespace === 'global' ? '' : ` --namespace ${namespace}` + return `npx @astron-team/skillhub@latest install ${slug}${namespaceArg} --registry ${baseUrl}` +} + +interface CommandBlockProps { + command: string +} + +const installMethodTabTriggerClass = + "relative border-b-0 px-1 py-2 text-xs after:absolute after:bottom-[-1px] after:left-1/2 after:h-0.5 after:w-6 after:-translate-x-1/2 after:rounded-full after:bg-transparent after:content-[''] data-[state=active]:after:bg-primary" + +function CommandBlock({ command }: CommandBlockProps) { const { t } = useTranslation() const [copied, copy] = useCopyToClipboard() - const baseUrl = useMemo(() => getBaseUrl(), []) - - const command = useMemo(() => buildInstallCommand(namespace, slug, baseUrl), [baseUrl, namespace, slug]) - const handleCopy = async () => { try { await copy(command) @@ -70,3 +79,29 @@ export function InstallCommand({ namespace, slug }: InstallCommandProps) { ) } + +export function InstallCommand({ namespace, slug }: InstallCommandProps) { + const { t } = useTranslation() + const baseUrl = useMemo(() => getBaseUrl(), []) + const clawhubCommand = useMemo(() => buildInstallCommand(namespace, slug, baseUrl), [baseUrl, namespace, slug]) + const skillhubCommand = useMemo(() => buildSkillhubInstallCommand(namespace, slug, baseUrl), [baseUrl, namespace, slug]) + + return ( + + + + {t('skillDetail.installMethodClawhub')} + + + {t('skillDetail.installMethodSkillhub')} + + + + + + + + + + ) +} diff --git a/web/src/features/skill/markdown-renderer.test.tsx b/web/src/features/skill/markdown-renderer.test.tsx index d7f39c4c..20fe3413 100644 --- a/web/src/features/skill/markdown-renderer.test.tsx +++ b/web/src/features/skill/markdown-renderer.test.tsx @@ -1,5 +1,10 @@ -import { describe, expect, it } from 'vitest' -import { MARKDOWN_IMAGE_CLASS_NAME } from './markdown-renderer' +/** @vitest-environment jsdom */ + +import { cleanup, fireEvent, render, screen } from '@testing-library/react' +import { afterEach, describe, expect, it, vi } from 'vitest' +import { MARKDOWN_IMAGE_CLASS_NAME, MarkdownRenderer } from './markdown-renderer' + +afterEach(() => cleanup()) describe('MARKDOWN_IMAGE_CLASS_NAME', () => { it('keeps markdown images at their intrinsic width while remaining responsive', () => { @@ -10,3 +15,21 @@ describe('MARKDOWN_IMAGE_CLASS_NAME', () => { expect(classNames).not.toContain('w-full') }) }) + +describe('MarkdownRenderer links', () => { + it('passes the raw markdown href to the optional link click handler', () => { + const onLinkClick = vi.fn() + + render() + fireEvent.click(screen.getByRole('link', { name: 'Usage' })) + + expect(onLinkClick).toHaveBeenCalledTimes(1) + expect(onLinkClick.mock.calls[0][0]).toBe('docs/usage.md') + }) + + it('keeps links renderable without a click handler', () => { + render() + + expect(screen.getByRole('link', { name: 'Usage' }).getAttribute('href')).toBe('docs/usage.md') + }) +}) diff --git a/web/src/features/skill/markdown-renderer.tsx b/web/src/features/skill/markdown-renderer.tsx index 1f787021..8bb0c198 100644 --- a/web/src/features/skill/markdown-renderer.tsx +++ b/web/src/features/skill/markdown-renderer.tsx @@ -1,4 +1,4 @@ -import { useMemo } from 'react' +import { useMemo, type MouseEvent } from 'react' import ReactMarkdown from 'react-markdown' import rehypeHighlight from 'rehype-highlight' import rehypeSanitize from 'rehype-sanitize' @@ -12,6 +12,7 @@ export const MARKDOWN_IMAGE_CLASS_NAME = 'h-auto max-w-full' interface MarkdownRendererProps { content: string className?: string + onLinkClick?: (href: string, event: MouseEvent) => void } /** @@ -20,7 +21,7 @@ interface MarkdownRendererProps { * dedicated UI sections and should not appear twice in the document body. * Memoized to prevent re-parsing on every render. */ -export function MarkdownRenderer({ content, className }: MarkdownRendererProps) { +export function MarkdownRenderer({ content, className, onLinkClick }: MarkdownRendererProps) { const containerClassName = [ className, 'max-w-none break-words text-sm text-foreground/90 [overflow-wrap:anywhere]', @@ -45,13 +46,15 @@ export function MarkdownRenderer({ content, className }: MarkdownRendererProps) {children}

), - a: ({ className: linkClassName, children, ...props }) => ( + a: ({ className: linkClassName, children, href, ...props }) => ( onLinkClick?.(href ?? '', event)} > {children} diff --git a/web/src/features/skill/package-relative-link.test.ts b/web/src/features/skill/package-relative-link.test.ts new file mode 100644 index 00000000..737be487 --- /dev/null +++ b/web/src/features/skill/package-relative-link.test.ts @@ -0,0 +1,80 @@ +import { describe, expect, it } from 'vitest' +import type { SkillFile } from '@/api/types' +import { resolvePackageRelativeLink } from './package-relative-link' + +function file(filePath: string): SkillFile { + return { + id: filePath.length, + filePath, + fileSize: 128, + contentType: 'text/markdown', + sha256: `sha-${filePath}`, + } +} + +const packageFiles = [ + file('README.md'), + file('docs/SKILL.md'), + file('docs/usage.md'), + file('shared.md'), + file('space name.md'), + file('使用.md'), +] + +describe('resolvePackageRelativeLink', () => { + it('matches same-directory and explicit current-directory links from the package root', () => { + expect(resolvePackageRelativeLink('docs/usage.md', 'README.md', packageFiles)).toMatchObject({ + status: 'matched', + path: 'docs/usage.md', + }) + expect(resolvePackageRelativeLink('./docs/usage.md', 'README.md', packageFiles)).toMatchObject({ + status: 'matched', + path: 'docs/usage.md', + }) + }) + + it('normalizes parent-directory links against the current documentation file', () => { + expect(resolvePackageRelativeLink('../shared.md', 'docs/SKILL.md', packageFiles)).toMatchObject({ + status: 'matched', + path: 'shared.md', + }) + }) + + it('keeps fragment information while matching the file path', () => { + expect(resolvePackageRelativeLink('docs/usage.md#intro', 'README.md', packageFiles)).toMatchObject({ + status: 'matched', + path: 'docs/usage.md', + fragment: 'intro', + }) + }) + + it('decodes encoded file paths before matching package files', () => { + expect(resolvePackageRelativeLink('space%20name.md', 'README.md', packageFiles)).toMatchObject({ + status: 'matched', + path: 'space name.md', + }) + expect(resolvePackageRelativeLink('%E4%BD%BF%E7%94%A8.md', 'README.md', packageFiles)).toMatchObject({ + status: 'matched', + path: '使用.md', + }) + }) + + it('ignores links that should keep native browser behavior', () => { + for (const href of ['https://example.com', 'mailto:team@example.com', '#intro', '/absolute/path.md', '']) { + expect(resolvePackageRelativeLink(href, 'README.md', packageFiles)).toMatchObject({ + status: 'ignored', + }) + } + }) + + it('returns missing for relative links that do not resolve to a package file', () => { + expect(resolvePackageRelativeLink('docs/missing.md', 'README.md', packageFiles)).toMatchObject({ + status: 'missing', + path: 'docs/missing.md', + }) + expect(resolvePackageRelativeLink('../../outside.md', 'docs/SKILL.md', packageFiles)).toMatchObject({ + status: 'missing', + path: null, + }) + }) +}) diff --git a/web/src/features/skill/package-relative-link.ts b/web/src/features/skill/package-relative-link.ts new file mode 100644 index 00000000..15c7bb25 --- /dev/null +++ b/web/src/features/skill/package-relative-link.ts @@ -0,0 +1,112 @@ +import type { SkillFile } from '@/api/types' + +export type PackageRelativeLinkResolution = + | { + status: 'ignored' + href: string + } + | { + status: 'matched' + href: string + path: string + fragment: string | null + file: SkillFile + } + | { + status: 'missing' + href: string + path: string | null + fragment: string | null + } + +function splitHref(href: string) { + const hashIndex = href.indexOf('#') + const beforeHash = hashIndex >= 0 ? href.slice(0, hashIndex) : href + const fragment = hashIndex >= 0 ? href.slice(hashIndex + 1) : null + const queryIndex = beforeHash.indexOf('?') + return { + path: queryIndex >= 0 ? beforeHash.slice(0, queryIndex) : beforeHash, + fragment, + } +} + +function decodePath(path: string) { + try { + return decodeURIComponent(path) + } catch { + return path + } +} + +function directoryOf(filePath?: string | null) { + if (!filePath) { + return '' + } + const normalized = filePath.replace(/^\/+/, '') + const lastSlash = normalized.lastIndexOf('/') + return lastSlash >= 0 ? normalized.slice(0, lastSlash) : '' +} + +function normalizePackagePath(baseDirectory: string, relativePath: string) { + const stack: string[] = [] + const rawParts = [...baseDirectory.split('/'), ...relativePath.split('/')] + + for (const part of rawParts) { + if (!part || part === '.') { + continue + } + if (part === '..') { + if (stack.length === 0) { + return null + } + stack.pop() + continue + } + stack.push(part) + } + + return stack.join('/') +} + +function shouldIgnoreLink(href: string, rawPath: string) { + if (!href.trim()) { + return true + } + if (!rawPath || href.startsWith('#')) { + return true + } + if (rawPath.startsWith('/') || rawPath.startsWith('//')) { + return true + } + return /^[a-z][a-z0-9+.-]*:/i.test(rawPath) +} + +export function resolvePackageRelativeLink( + href: string, + currentFilePath: string | null | undefined, + files: SkillFile[] | null | undefined, +): PackageRelativeLinkResolution { + const { path: rawPath, fragment } = splitHref(href) + + if (shouldIgnoreLink(href, rawPath)) { + return { status: 'ignored', href } + } + + const normalizedPath = normalizePackagePath(directoryOf(currentFilePath), decodePath(rawPath)) + if (!normalizedPath) { + return { status: 'missing', href, path: null, fragment } + } + + const matchedFile = (files ?? []).find((file) => file.filePath === normalizedPath) + if (!matchedFile) { + return { status: 'missing', href, path: normalizedPath, fragment } + } + + return { + status: 'matched', + href, + path: normalizedPath, + fragment, + file: matchedFile, + } +} diff --git a/web/src/i18n/locales/en.json b/web/src/i18n/locales/en.json index 3ff964a4..b63da59f 100644 --- a/web/src/i18n/locales/en.json +++ b/web/src/i18n/locales/en.json @@ -339,6 +339,12 @@ "mySkills": { "title": "My Skills", "subtitle": "Manage your published skills", + "searchPlaceholder": "Search by name, slug, or description", + "namespaceFilterLabel": "Filter by namespace", + "namespaceFilterAll": "All namespaces", + "clearSearch": "Clear filters", + "emptySearchTitle": "No matching skills", + "emptySearchDescription": "Try adjusting your keyword or switching namespace.", "filters": { "ALL": "All", "PENDING_REVIEW": "Pending Review", @@ -784,6 +790,8 @@ "documentationSource": "Source: {{path}}", "documentationUnavailableTitle": "Documentation is unavailable", "documentationUnavailable": "The documentation file could not be loaded. You can still inspect the package contents in the file list.", + "packageLinkMissingTitle": "File not found", + "packageLinkMissingDescription": "This link points to a file that is not included in the current skill version.", "authorLabel": "By {{name}}", "expandOverview": "Expand full overview", "collapseOverview": "Collapse content", @@ -801,6 +809,8 @@ "namespaceLabel": "Namespace", "loginToRate": "Login to star and rate", "install": "Install", + "installMethodClawhub": "ClawHub CLI", + "installMethodSkillhub": "SkillHub CLI", "download": "Download", "labelsSectionTitle": "Labels", "labelsSectionDescription": "Attach or remove recommended labels that help users filter and discover this skill.", @@ -1277,7 +1287,8 @@ "prev": "Previous", "next": "Next", "pagePrefix": "Page", - "pageSuffix": "" + "pageSuffix": "", + "goToPage": "Go to page {{page}}" }, "user": { "menu": { diff --git a/web/src/i18n/locales/zh.json b/web/src/i18n/locales/zh.json index 85cdc73b..243ec719 100644 --- a/web/src/i18n/locales/zh.json +++ b/web/src/i18n/locales/zh.json @@ -339,6 +339,12 @@ "mySkills": { "title": "我的技能", "subtitle": "管理你发布的技能", + "searchPlaceholder": "搜索技能名称、Slug 或描述", + "namespaceFilterLabel": "按命名空间过滤", + "namespaceFilterAll": "全部命名空间", + "clearSearch": "清除筛选", + "emptySearchTitle": "未找到匹配的技能", + "emptySearchDescription": "试试调整关键字或切换命名空间", "filters": { "ALL": "全部", "PENDING_REVIEW": "待审核", @@ -784,6 +790,8 @@ "documentationSource": "来源:{{path}}", "documentationUnavailableTitle": "文档暂时不可用", "documentationUnavailable": "当前无法读取这个技能版本的文档文件。你仍然可以在文件列表里查看包内容。", + "packageLinkMissingTitle": "文件未找到", + "packageLinkMissingDescription": "该链接指向的文件不在当前技能版本中。", "authorLabel": "作者 {{name}}", "expandOverview": "展开全文", "collapseOverview": "收起内容", @@ -801,6 +809,8 @@ "namespaceLabel": "命名空间", "loginToRate": "登录后可以收藏和评分", "install": "安装", + "installMethodClawhub": "ClawHub CLI", + "installMethodSkillhub": "SkillHub CLI", "download": "下载", "labelsSectionTitle": "标签管理", "labelsSectionDescription": "为这个技能挂载或移除推荐标签,帮助用户筛选和发现。", @@ -1278,7 +1288,8 @@ "prev": "上一页", "next": "下一页", "pagePrefix": "第", - "pageSuffix": "页" + "pageSuffix": "页", + "goToPage": "第 {{page}} 页" }, "user": { "menu": { diff --git a/web/src/i18n/skill-detail-locale.test.ts b/web/src/i18n/skill-detail-locale.test.ts index 6434f4d5..a237ef60 100644 --- a/web/src/i18n/skill-detail-locale.test.ts +++ b/web/src/i18n/skill-detail-locale.test.ts @@ -7,4 +7,11 @@ describe('skill detail lifecycle locales', () => { expect(zh.skillDetail.unarchiveSkill).toBe('恢复技能') expect(en.skillDetail.unarchiveSkill).toBe('Restore Skill') }) + + it('defines package relative link missing messages in both locales', () => { + expect(zh.skillDetail.packageLinkMissingTitle).toBe('文件未找到') + expect(zh.skillDetail.packageLinkMissingDescription).toBe('该链接指向的文件不在当前技能版本中。') + expect(en.skillDetail.packageLinkMissingTitle).toBe('File not found') + expect(en.skillDetail.packageLinkMissingDescription).toBe('This link points to a file that is not included in the current skill version.') + }) }) diff --git a/web/src/pages/dashboard/my-skills.test.ts b/web/src/pages/dashboard/my-skills.test.ts index f2c58112..c82faf93 100644 --- a/web/src/pages/dashboard/my-skills.test.ts +++ b/web/src/pages/dashboard/my-skills.test.ts @@ -8,6 +8,8 @@ const useMySkillsMock = vi.fn() vi.mock('@tanstack/react-router', () => ({ useNavigate: () => navigateMock, + useLocation: () => ({ pathname: '/dashboard/skills' }), + useSearch: () => ({}), })) vi.mock('react-i18next', async () => { @@ -69,6 +71,14 @@ vi.mock('@/shared/hooks/use-user-queries', () => ({ useSubmitPromotion: () => ({ mutateAsync: vi.fn(), isPending: false }), })) +vi.mock('@/shared/hooks/use-namespace-queries', () => ({ + useMyNamespaces: () => ({ data: [] }), +})) + +vi.mock('@/shared/hooks/use-debounce', () => ({ + useDebounce: (value: string) => value, +})) + vi.mock('@/shared/lib/skill-lifecycle', () => ({ getHeadlineVersion: () => ({ id: 11, version: '1.0.0', status: 'PUBLISHED' }), getPublishedVersion: () => ({ id: 11, version: '1.0.0', status: 'PUBLISHED' }), diff --git a/web/src/pages/dashboard/my-skills.tsx b/web/src/pages/dashboard/my-skills.tsx index 78192c32..0d90f928 100644 --- a/web/src/pages/dashboard/my-skills.tsx +++ b/web/src/pages/dashboard/my-skills.tsx @@ -1,22 +1,28 @@ -import { useState } from 'react' -import { useNavigate } from '@tanstack/react-router' +import { useEffect, useState } from 'react' +import { useLocation, useNavigate, useSearch } from '@tanstack/react-router' import { useTranslation } from 'react-i18next' import { useAuth } from '@/features/auth/use-auth' import { Button } from '@/shared/ui/button' import { Card } from '@/shared/ui/card' +import { Input } from '@/shared/ui/input' +import { Select, SelectContent, SelectItem, SelectTrigger, SelectValue } from '@/shared/ui/select' import { EmptyState } from '@/shared/components/empty-state' import { ConfirmDialog } from '@/shared/components/confirm-dialog' import { DashboardPageHeader } from '@/shared/components/dashboard-page-header' import { Pagination } from '@/shared/components/pagination' import { useArchiveSkill, useUnarchiveSkill, useWithdrawSkillReview } from '@/shared/hooks/use-skill-queries' +import { useMyNamespaces } from '@/shared/hooks/use-namespace-queries' import { useMySkills, useSubmitPromotion } from '@/shared/hooks/use-user-queries' +import { useDebounce } from '@/shared/hooks/use-debounce' import { getHeadlineVersion, getPublishedVersion, getOwnerPreviewVersion, hasPendingOwnerPreview } from '@/shared/lib/skill-lifecycle' import { formatCompactCount } from '@/shared/lib/number-format' import { toast } from '@/shared/lib/toast' +import { buildReturnTo } from '@/shared/lib/auth-route' import { ApiError } from '@/api/client' import { getMySkillEmptyStateKey, getMySkillFilters, type MySkillFilter } from './my-skill-filters' const PAGE_SIZE = 10 +const ALL_NAMESPACES_VALUE = '__all_namespaces__' /** * Dashboard page for skills owned by the current user. @@ -36,18 +42,61 @@ function getPromotionConflictKey(error: ApiError): 'promotion.duplicate_pending' export function MySkillsPage() { const navigate = useNavigate() + const location = useLocation() + const search = useSearch({ from: '/dashboard/skills' }) const { t } = useTranslation() const { hasRole } = useAuth() - const [page, setPage] = useState(0) - const [filter, setFilter] = useState('ALL') + + // The URL is the source of truth for page / filter / namespace / keyword so the + // search context survives navigating into a skill and back via the returnTo link. + const page = search.page ?? 0 + const filter = (search.filter as MySkillFilter) ?? 'ALL' + const namespaceFilter = search.namespace ?? '' + const keyword = search.q ?? '' + + // Keep an instant-feedback copy of the keyword input, debounced before it is + // pushed to the URL so each keystroke does not create a history entry or query. + const [keywordInput, setKeywordInput] = useState(keyword) + const debouncedKeyword = useDebounce(keywordInput.trim(), 300) + const [archiveTarget, setArchiveTarget] = useState<{ namespace: string; slug: string; name: string } | null>(null) const [unarchiveTarget, setUnarchiveTarget] = useState<{ namespace: string; slug: string; name: string } | null>(null) const [withdrawTarget, setWithdrawTarget] = useState<{ namespace: string; slug: string; name: string; version: string } | null>(null) const [promotionTarget, setPromotionTarget] = useState<{ skillId: number; versionId: number; name: string; version: string } | null>(null) - const { data: skillPage, isLoading } = useMySkills({ page, size: PAGE_SIZE, filter: filter === 'ALL' ? undefined : filter }) + + const updateSearch = (next: Partial, options?: { replace?: boolean }) => { + navigate({ + to: '/dashboard/skills', + search: (prev) => ({ ...prev, ...next }), + replace: options?.replace, + }) + } + + // Push the debounced keyword to the URL (reset page to 0 when search changes) + useEffect(() => { + if (debouncedKeyword !== keyword) { + updateSearch({ q: debouncedKeyword || undefined, page: 0 }, { replace: true }) + } + }, [debouncedKeyword]) + + // Sync keywordInput when navigating back via returnTo + useEffect(() => { + setKeywordInput(keyword) + }, [keyword]) + + const { data: skillPage, isLoading } = useMySkills({ + page, + size: PAGE_SIZE, + filter: filter === 'ALL' ? undefined : filter, + q: keyword || undefined, + namespace: namespaceFilter || undefined, + }) + const { data: namespaceOptions } = useMyNamespaces() + const skills = skillPage?.items ?? [] const totalPages = skillPage ? Math.max(Math.ceil(skillPage.total / skillPage.size), 1) : 1 const availableFilters = getMySkillFilters(hasRole('SUPER_ADMIN')) + const hasActiveSearch = keyword.trim() !== '' || namespaceFilter !== '' const emptyStateKey = getMySkillEmptyStateKey(filter) const archiveMutation = useArchiveSkill() const unarchiveMutation = useUnarchiveSkill() @@ -57,10 +106,15 @@ export function MySkillsPage() { const handleSkillClick = (namespace: string, slug: string) => { navigate({ to: `/space/${namespace}/${encodeURIComponent(slug)}`, - search: { returnTo: '/dashboard/skills' }, + search: { returnTo: buildReturnTo(location) }, }) } + const handleClearSearch = () => { + setKeywordInput('') + updateSearch({ q: undefined, namespace: undefined, page: 0 }) + } + const handleUpdateSkill = (namespace: string, visibility?: string) => { navigate({ to: '/dashboard/publish', @@ -238,21 +292,61 @@ export function MySkillsPage() { )} /> -
- {availableFilters.map((option) => ( - + ) : null} +
+ +
+ {availableFilters.map((option) => ( + + ))} +
{skillPage && skillPage.total > 0 ? ( @@ -400,17 +494,23 @@ export function MySkillsPage() { {skillPage.total > PAGE_SIZE ? ( - + updateSearch({ page: next })} /> ) : null} ) : ( navigate({ to: '/dashboard/publish' })}> - {t('mySkills.publishSkill')} - + hasActiveSearch ? ( + + ) : ( + + ) } /> )} diff --git a/web/src/pages/search.tsx b/web/src/pages/search.tsx index 58c421db..a0865909 100644 --- a/web/src/pages/search.tsx +++ b/web/src/pages/search.tsx @@ -186,7 +186,7 @@ export function SearchPage() { } const handleSkillClick = (namespace: string, slug: string) => { - navigate({ to: `/space/${namespace}/${encodeURIComponent(slug)}` }) + navigate({ to: `/space/${namespace}/${encodeURIComponent(slug)}`, search: { returnTo: `${window.location.pathname}${window.location.search}` } }) } const filteredStarredSkills = starredOnly diff --git a/web/src/pages/skill-detail.test.tsx b/web/src/pages/skill-detail.test.tsx index 87bdcb48..1c6374ce 100644 --- a/web/src/pages/skill-detail.test.tsx +++ b/web/src/pages/skill-detail.test.tsx @@ -1,11 +1,24 @@ +/** @vitest-environment jsdom */ + import { renderToStaticMarkup } from 'react-dom/server' -import { beforeEach, describe, expect, it, vi } from 'vitest' +import { cleanup, fireEvent, render, screen } from '@testing-library/react' +import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest' +import type { MouseEvent } from 'react' +import type { SkillFile } from '@/api/types' + +const toastMocks = vi.hoisted(() => ({ + success: vi.fn(), + error: vi.fn(), +})) const navigateMock = vi.fn() const hasRoleMock = vi.fn<(role: string) => boolean>((role: string) => role === 'USER') const useSkillDetailMock = vi.fn() const useSkillLabelsMock = vi.fn() const useSkillVersionsMock = vi.fn() +const useSkillFilesMock = vi.fn() +const useSkillReadmeMock = vi.fn() +const useSkillFileMock = vi.fn() let authState: { user: { userId: string; platformRoles: string[] } | null hasRole: (role: string) => boolean @@ -47,7 +60,7 @@ vi.mock('@/features/report/use-skill-reports', () => ({ })) vi.mock('@/shared/lib/toast', () => ({ - toast: { success: vi.fn(), error: vi.fn() }, + toast: { success: toastMocks.success, error: toastMocks.error }, })) vi.mock('@/api/client', () => ({ @@ -76,7 +89,47 @@ vi.mock('@/shared/lib/number-format', () => ({ })) vi.mock('@/features/skill/markdown-renderer', () => ({ - MarkdownRenderer: () =>
markdown
, + MarkdownRenderer: ({ + content, + onLinkClick, + }: { + content: string + onLinkClick?: (href: string, event: MouseEvent) => void + }) => ( + + ), +})) + +vi.mock('@/features/skill/file-preview-dialog', () => ({ + FilePreviewDialog: ({ open, node }: { open: boolean; node: { path: string } | null }) => ( + open && node ?
preview:{node.path}
: null + ), })) vi.mock('@/features/skill/file-tree', () => ({ @@ -107,9 +160,9 @@ vi.mock('@/shared/hooks/use-skill-queries', () => ({ useDetachSkillLabel: () => ({ mutate: vi.fn(), isPending: false }), useSkillVersions: (...args: unknown[]) => useSkillVersionsMock(...args), useSkillVersionDetail: () => ({ data: undefined }), - useSkillFiles: () => ({ data: [] }), - useSkillReadme: () => ({ data: '# Demo', error: null }), - useSkillFile: () => ({ data: null, isLoading: false, error: null }), + useSkillFiles: (...args: unknown[]) => useSkillFilesMock(...args), + useSkillReadme: (...args: unknown[]) => useSkillReadmeMock(...args), + useSkillFile: (...args: unknown[]) => useSkillFileMock(...args), useArchiveSkill: () => ({ mutateAsync: vi.fn(), isPending: false }), useDeleteSkill: () => ({ mutateAsync: vi.fn(), isPending: false }), useDeleteSkillVersion: () => ({ mutateAsync: vi.fn(), isPending: false }), @@ -165,9 +218,26 @@ function createSkill(overrides: Record = {}) { } } +function createSkillFile(filePath: string): SkillFile { + return { + id: filePath.length, + filePath, + fileSize: 128, + contentType: 'text/markdown', + sha256: `sha-${filePath}`, + } +} + describe('SkillDetailPage', () => { + afterEach(() => cleanup()) + beforeEach(() => { navigateMock.mockReset() + useSkillFilesMock.mockReset() + useSkillReadmeMock.mockReset() + useSkillFileMock.mockReset() + toastMocks.success.mockReset() + toastMocks.error.mockReset() hasRoleMock.mockImplementation((role: string) => role === 'USER') authState = { user: { userId: 'owner-1', platformRoles: ['USER'] }, @@ -196,6 +266,9 @@ describe('SkillDetailPage', () => { useSkillLabelsMock.mockReturnValue({ data: undefined, }) + useSkillFilesMock.mockReturnValue({ data: [] }) + useSkillReadmeMock.mockReturnValue({ data: '# Demo', error: null }) + useSkillFileMock.mockReturnValue({ data: null, isLoading: false, error: null }) }) it('shows hard delete action for the skill owner', () => { @@ -401,4 +474,53 @@ describe('SkillDetailPage', () => { expect(html).toContain('break-all') expect(html).toContain('leading-snug') }) + + it('opens a file preview when overview markdown relative link matches a package file', () => { + useSkillFilesMock.mockReturnValue({ + data: [ + createSkillFile('README.md'), + createSkillFile('docs/usage.md'), + ], + }) + + render() + fireEvent.click(screen.getByRole('link', { name: 'Usage' })) + + expect(screen.getByRole('dialog').textContent).toContain('preview:docs/usage.md') + expect(toastMocks.error).not.toHaveBeenCalled() + }) + + it('keeps the viewer on the detail page and shows a toast for missing package files', () => { + useSkillFilesMock.mockReturnValue({ + data: [ + createSkillFile('README.md'), + createSkillFile('docs/usage.md'), + ], + }) + + render() + fireEvent.click(screen.getByRole('link', { name: 'Missing' })) + + expect(screen.queryByRole('dialog')).toBeNull() + expect(toastMocks.error).toHaveBeenCalledWith( + 'skillDetail.packageLinkMissingTitle', + 'skillDetail.packageLinkMissingDescription', + ) + }) + + it('leaves external links and same-document anchors alone', () => { + useSkillFilesMock.mockReturnValue({ + data: [ + createSkillFile('README.md'), + createSkillFile('docs/usage.md'), + ], + }) + + render() + fireEvent.click(screen.getByRole('link', { name: 'External' })) + fireEvent.click(screen.getByRole('link', { name: 'Anchor' })) + + expect(screen.queryByRole('dialog')).toBeNull() + expect(toastMocks.error).not.toHaveBeenCalled() + }) }) diff --git a/web/src/pages/skill-detail.tsx b/web/src/pages/skill-detail.tsx index 086f85a0..77b0c291 100644 --- a/web/src/pages/skill-detail.tsx +++ b/web/src/pages/skill-detail.tsx @@ -1,12 +1,14 @@ -import { useEffect, useRef, useState } from 'react' +import { useEffect, useRef, useState, type MouseEvent } from 'react' import { useTranslation } from 'react-i18next' import { useParams, useNavigate, useRouterState, useSearch } from '@tanstack/react-router' import { useMutation, useQueryClient } from '@tanstack/react-query' import { ArrowLeft, ArrowUpCircle, ChevronDown, ChevronUp, Clock, Folder, Globe, Lock, RefreshCw, ShieldCheck, Terminal, User, Users } from 'lucide-react' import { MarkdownRenderer } from '@/features/skill/markdown-renderer' +import { resolvePackageRelativeLink } from '@/features/skill/package-relative-link' import { FileTree } from '@/features/skill/file-tree' import { FilePreviewDialog } from '@/features/skill/file-preview-dialog' import type { FileTreeNode } from '@/features/skill/file-tree-builder' +import type { SkillFile } from '@/api/types' import { InstallCommand } from '@/features/skill/install-command' import { ShareButton } from '@/features/skill/share-button' import { SkillLabelPanel } from '@/features/skill/skill-label-panel' @@ -87,6 +89,20 @@ function parseMetadataJson(parsed?: string) { } } +function createPackageFilePreviewNode(file: SkillFile): FileTreeNode { + const pathParts = file.filePath.split('/').filter(Boolean) + const name = pathParts[pathParts.length - 1] ?? file.filePath + + return { + id: file.filePath, + name, + path: file.filePath, + type: 'file', + file, + depth: Math.max(pathParts.length - 1, 0), + } +} + function getPromotionConflictKey(error: ApiError): 'promotion.duplicate_pending' | 'promotion.already_promoted' | null { if (error.serverMessageKey === 'promotion.duplicate_pending') { return 'promotion.duplicate_pending' @@ -172,7 +188,6 @@ export function SkillDetailPage() { && ['PENDING_REVIEW', 'SCANNING', 'SCAN_FAILED'].includes(headlineVersion?.status ?? '') const hasPendingOwnerPreview = ownerPreviewVersion?.status === 'PENDING_REVIEW' const hasRejectedOwnerPreview = ownerPreviewVersion?.status === 'REJECTED' - const hasRejectedVersion = versions?.some((v) => v.status === 'REJECTED') ?? false const hasPublishedPendingReview = Boolean(publishedVersion && hasPendingOwnerPreview) const canInteract = skill?.canInteract ?? true const canReport = skill?.canReport ?? true @@ -285,6 +300,24 @@ export function SkillDetailPage() { setPreviewDialogOpen(true) } + const handleOverviewLinkClick = (href: string, event: MouseEvent) => { + const resolution = resolvePackageRelativeLink(href, documentationPath, files) + + if (resolution.status === 'ignored') { + return + } + + event.preventDefault() + + if (resolution.status === 'matched') { + setPreviewNode(createPackageFilePreviewNode(resolution.file)) + setPreviewDialogOpen(true) + return + } + + toast.error(t('skillDetail.packageLinkMissingTitle'), t('skillDetail.packageLinkMissingDescription')) + } + // Download a single file from the skill version const handleDownloadFile = () => { const isAnonymousAllowed = namespace === 'global' && skill?.visibility === 'PUBLIC' @@ -762,7 +795,7 @@ export function SkillDetailPage() { {t('skillDetail.versionStatusPendingReview')} )} - {!isPendingPreview && (isRejectedPreview || hasRejectedOwnerPreview || hasRejectedVersion) && skill.canManageLifecycle && ( + {!isPendingPreview && (isRejectedPreview || hasRejectedOwnerPreview) && skill.canManageLifecycle && ( {t('skillDetail.rejectedBadge')} @@ -845,7 +878,7 @@ export function SkillDetailPage() { style={!isOverviewExpanded && isOverviewCollapsible ? { maxHeight: `${overviewMaxHeight}px` } : undefined} >
- +
{!isOverviewExpanded && isOverviewCollapsible ? (
diff --git a/web/src/shared/components/pagination.tsx b/web/src/shared/components/pagination.tsx index 5f54d904..069dbd85 100644 --- a/web/src/shared/components/pagination.tsx +++ b/web/src/shared/components/pagination.tsx @@ -7,8 +7,43 @@ interface PaginationProps { onPageChange: (page: number) => void } +type PageItem = number | 'ellipsis' + +/** + * Builds the list of page slots to render. Always shows the first and last page, + * the current page, and one neighbour on each side, collapsing the rest into + * ellipsis markers. Pages are 0-indexed internally; labels are 1-indexed. + */ +function buildPageItems(current: number, totalPages: number): PageItem[] { + if (totalPages <= 7) { + return Array.from({ length: totalPages }, (_, i) => i) + } + + const items: PageItem[] = [] + const first = 0 + const last = totalPages - 1 + const start = Math.max(first + 1, current - 1) + const end = Math.min(last - 1, current + 1) + + items.push(first) + if (start > first + 1) { + items.push('ellipsis') + } + for (let i = start; i <= end; i += 1) { + items.push(i) + } + if (end < last - 1) { + items.push('ellipsis') + } + items.push(last) + + return items +} + export function Pagination({ page, totalPages, onPageChange }: PaginationProps) { const { t } = useTranslation() + const pageItems = buildPageItems(page, totalPages) + return (
-
- {t('pagination.pagePrefix')} - {page + 1} - / - {totalPages} - {t('pagination.pageSuffix') && {t('pagination.pageSuffix')}} + +
+ {pageItems.map((item, index) => + item === 'ellipsis' ? ( + + ) : ( + + ), + )}
+